[PATCH] clk: debugfs add clk_rate_fops with DEBUG

[PATCH] clk: debugfs add clk_rate_fops with DEBUG

[Zhangfei Gao]

clk_rate_fops is added to debug
1. set_rate (e.g. PLL)
2. Choose mux parent, since mux could choose parent accordingly when set_rate.

Causion: set_rate can be called directly from user space

Example:
sfc_mux have two parents: 24M and 200M

cat clk_summary
   clock                        enable_cnt  prepare_cnt  rate
---------------------------------------------------------------------
 osc24mhz                       3           3            24000000
    bpll_fout3                  0           0            200000000
       sfc_mux                  0           0            200000000
          sfc                   0           0            200000000

cat osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
200000000

echo 24000000 >  osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
cat clk_summary
   clock                        enable_cnt  prepare_cnt  rate
---------------------------------------------------------------------
 osc24mhz                       3           3            24000000
    sfc_mux                     0           0            24000000
       sfc                      0           0            24000000
    bpll_fout3                  0           0            200000000

cat osc24mhz/sfc_mux/sfc/clk_rate
24000000

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/clk/clk.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 373cd54..6b582f7 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -226,6 +226,35 @@ static const struct file_operations clk_dump_fops = {
 	.release	= single_release,
 };
 
+#ifdef DEBUG
+static int clk_rate_fops_get(void *data, u64 *rate)
+{
+	struct clk *clk = data;
+
+	*rate = clk->rate;
+
+	return 0;
+};
+
+static int clk_rate_fops_set(void *data, u64 rate)
+{
+	struct clk *clk = data;
+	int ret = 0;
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto out;
+	clk_set_rate(clk, rate);
+	clk_disable_unprepare(clk);
+
+out:
+	return ret;
+};
+
+DEFINE_SIMPLE_ATTRIBUTE(clk_rate_fops, clk_rate_fops_get,
+			clk_rate_fops_set, "%llu\n");
+#endif
+
 /* caller must hold prepare_lock */
 static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
 {
@@ -243,8 +272,13 @@ static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
 
 	clk->dentry = d;
 
+#ifdef DEBUG
+	d = debugfs_create_file("clk_rate", S_IWUSR | S_IRUGO, clk->dentry,
+				clk, &clk_rate_fops);
+#else
 	d = debugfs_create_u32("clk_rate", S_IRUGO, clk->dentry,
 			(u32 *)&clk->rate);
+#endif
 	if (!d)
 		goto err_out;
 
-- 
1.7.9.5

[PATCH] clk: debugfs add clk_rate_fops with DEBUG

[Mike Turquette]

Quoting Zhangfei Gao (2013-08-18 22:34:17)
> clk_rate_fops is added to debug
> 1. set_rate (e.g. PLL)
> 2. Choose mux parent, since mux could choose parent accordingly when set_rate.
> 
> Causion: set_rate can be called directly from user space
> 
> Example:
> sfc_mux have two parents: 24M and 200M
> 
> cat clk_summary
>    clock                        enable_cnt  prepare_cnt  rate
> ---------------------------------------------------------------------
>  osc24mhz                       3           3            24000000
>     bpll_fout3                  0           0            200000000
>        sfc_mux                  0           0            200000000
>           sfc                   0           0            200000000
> 
> cat osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
> 200000000
> 
> echo 24000000 >  osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
> cat clk_summary
>    clock                        enable_cnt  prepare_cnt  rate
> ---------------------------------------------------------------------
>  osc24mhz                       3           3            24000000
>     sfc_mux                     0           0            24000000
>        sfc                      0           0            24000000
>     bpll_fout3                  0           0            200000000
> 
> cat osc24mhz/sfc_mux/sfc/clk_rate
> 24000000
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>

Thanks for the patch. It is trivial enough where I think we can leave it
on the list and let developers apply it if they need it for debug. I
don't want to merge it since it exposes hardware control to userspace
and could be abused in a way that could damage a device.

Regards,
Mike

> ---
>  drivers/clk/clk.c |   34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 373cd54..6b582f7 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -226,6 +226,35 @@ static const struct file_operations clk_dump_fops = {
>         .release        = single_release,
>  };
>  
> +#ifdef DEBUG
> +static int clk_rate_fops_get(void *data, u64 *rate)
> +{
> +       struct clk *clk = data;
> +
> +       *rate = clk->rate;
> +
> +       return 0;
> +};
> +
> +static int clk_rate_fops_set(void *data, u64 rate)
> +{
> +       struct clk *clk = data;
> +       int ret = 0;
> +
> +       ret = clk_prepare_enable(clk);
> +       if (ret)
> +               goto out;
> +       clk_set_rate(clk, rate);
> +       clk_disable_unprepare(clk);
> +
> +out:
> +       return ret;
> +};
> +
> +DEFINE_SIMPLE_ATTRIBUTE(clk_rate_fops, clk_rate_fops_get,
> +                       clk_rate_fops_set, "%llu\n");
> +#endif
> +
>  /* caller must hold prepare_lock */
>  static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
>  {
> @@ -243,8 +272,13 @@ static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
>  
>         clk->dentry = d;
>  
> +#ifdef DEBUG
> +       d = debugfs_create_file("clk_rate", S_IWUSR | S_IRUGO, clk->dentry,
> +                               clk, &clk_rate_fops);
> +#else
>         d = debugfs_create_u32("clk_rate", S_IRUGO, clk->dentry,
>                         (u32 *)&clk->rate);
> +#endif
>         if (!d)
>                 goto err_out;
>  
> -- 
> 1.7.9.5

[PATCH] clk: debugfs add clk_rate_fops with DEBUG

[zhangfei gao]

On Tue, Oct 8, 2013 at 5:29 AM, Mike Turquette <mturquette@linaro.org> wrote:
> Quoting Zhangfei Gao (2013-08-18 22:34:17)
>> clk_rate_fops is added to debug
>> 1. set_rate (e.g. PLL)
>> 2. Choose mux parent, since mux could choose parent accordingly when set_rate.
>>
>> Causion: set_rate can be called directly from user space
>>
>> Example:
>> sfc_mux have two parents: 24M and 200M
>>
>> cat clk_summary
>>    clock                        enable_cnt  prepare_cnt  rate
>> ---------------------------------------------------------------------
>>  osc24mhz                       3           3            24000000
>>     bpll_fout3                  0           0            200000000
>>        sfc_mux                  0           0            200000000
>>           sfc                   0           0            200000000
>>
>> cat osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
>> 200000000
>>
>> echo 24000000 >  osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate
>> cat clk_summary
>>    clock                        enable_cnt  prepare_cnt  rate
>> ---------------------------------------------------------------------
>>  osc24mhz                       3           3            24000000
>>     sfc_mux                     0           0            24000000
>>        sfc                      0           0            24000000
>>     bpll_fout3                  0           0            200000000
>>
>> cat osc24mhz/sfc_mux/sfc/clk_rate
>> 24000000
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>
> Thanks for the patch. It is trivial enough where I think we can leave it
> on the list and let developers apply it if they need it for debug. I
> don't want to merge it since it exposes hardware control to userspace
> and could be abused in a way that could damage a device.
>
> Regards,
> Mike
>

Thanks Mike

Understand the risk, it is fine.

Thanks