[PATCH] clocks: add clock debugging file

Jeremy Kerr jeremy.kerr at canonical.com
Wed Nov 17 06:02:01 UTC 2010


Hi Yong,

A few more comments:

> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
> index 0a34c81..0300c95 100644
> --- a/arch/arm/common/Kconfig
> +++ b/arch/arm/common/Kconfig
> @@ -41,3 +41,10 @@ config SHARP_SCOOP
>  config COMMON_CLKDEV
>   bool
>   select HAVE_CLK
> +
> +config CLK_DEBUG
> + bool "clock debug information export to user space"
> + depends on COMMON_CLKDEV && PM_DEBUG && DEBUG_FS

You need to depend on USE_COMMON_STRUCT_CLK (instead of COMMON_CLKDEV) here.

> + default n
> + help
> +  export clk debug information to user space
> diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c
> index 9e4c4d9..5f4a309 100644
> --- a/arch/arm/common/clkdev.c
> +++ b/arch/arm/common/clkdev.c

This is still arm-specific? Looks like you've moved some of the changes to 
kernel/clk.c, but not all of them.

The only change we should need to the ARM code is to call clk_debug_register 
in the clkdev_add and clkdev_add_table functions. See my original patch for 
details.


> diff --git a/arch/arm/plat-mxc/clock-common.c
> b/arch/arm/plat-mxc/clock-common.c
> index 8911813..20d38a8 100644
> --- a/arch/arm/plat-mxc/clock-common.c
> +++ b/arch/arm/plat-mxc/clock-common.c
> @@ -97,7 +97,10 @@ unsigned long clk_mxc_get_rate(struct clk *_clk)
>   if (clk->get_rate)
>   return clk->get_rate(clk);
> 
> - return clk_get_rate(clk->parent);
> + if (clk->parent)
> + return clk_get_rate(clk->parent);
> +
> + return 0;
>  }
> 
>  /* Round the requested clock rate to the nearest supported

We should probably move this to a separate patch.

> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 56416b7..751c086 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -44,16 +44,28 @@ struct device;
>   * registered with the arch-specific clock management code; the clock
> driver
>   * code does not need to handle these.
>   */
> +#define CLK_NAME_LEN 32
>  struct clk {
>   const struct clk_ops *ops;
>   unsigned int enable_count;
>   struct mutex mutex;
> +#ifdef CONFIG_CLK_DEBUG
> + char name[CLK_NAME_LEN];
> + struct dentry           *dentry;
> +#endif
>  };
> 
> -#define INIT_CLK(name, o) { \
> +#ifdef CONFIG_CLK_DEBUG
> +#define __INIT_CLK_DEBUG(n) .name = #n,
> +#else
> +#define __INIT_CLK_DEBUG(n)
> +#endif
> +
> +#define INIT_CLK(clk_name, o) { \
>   .ops = &o, \
>   .enable_count = 0, \
> - .mutex = __MUTEX_INITIALIZER(name.mutex), \
> + .mutex = __MUTEX_INITIALIZER(clk_name.mutex), \
> + __INIT_CLK_DEBUG(clk_name) \
>  }

Looks better, but you don't need to change the macro argument to clk_name 
here.

> 
>  struct clk_ops {
> @@ -245,4 +257,7 @@ struct clk *clk_get_sys(const char *dev_id, const char
> *con_id);
>  int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,
>   struct device *dev);
> 
> +#ifdef CONFIG_CLK_DEBUG
> +int clk_debugfs_register(struct clk *clk, struct dentry *root);
> +#endif

You'll need to provide an empty declaration for the !CONFIG_CLK_DEBUG case. 
Also, I'd probably prefer clk_debug_* rather than clk_debugfs_*; we may add 
functionality that isn't tied to debugfs in future.


>  #endif
> diff --git a/kernel/clk.c b/kernel/clk.c
> index 32f25ef..97e5e66 100644
> --- a/kernel/clk.c
> +++ b/kernel/clk.c
> @@ -12,6 +12,10 @@
>  #include <linux/mutex.h>
>  #include <linux/module.h>
> 
> +#ifdef CONFIG_CLK_DEBUG
> +#include <linux/debugfs.h>
> +#endif

Don't worry about making this conditional on CONFIG_CLK_DEBUG.

Cheers,


Jeremy



More information about the linaro-dev mailing list