All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] clk: Fix debugfs_create_*() usage
@ 2018-01-03 11:06 Geert Uytterhoeven
  2018-01-03 11:06 ` [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical() Geert Uytterhoeven
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-03 11:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-kernel, Geert Uytterhoeven

	Hi Mike, Stephen,

When exposing data access through debugfs, the correct
debugfs_create_*() functions must be used, matching the data types.

This patch series remove all casts from data pointers passed to
debugfs_create_*() functions, as such casts prevent the compiler from flagging
bugs, and fixes wrong usage exposed by that.

Please refer to the individual patches for a changelog.

Thanks!

Geert Uytterhoeven (4):
  clk: Improve flags doc for of_clk_detect_critical()
  clk: Use octal instead of symbolic permissions
  clk: Show symbolic clock flags in debugfs
  clk: Fix debugfs_create_*() usage

 drivers/clk/clk.c            | 92 +++++++++++++++++++++++++++++++++-----------
 include/linux/clk-provider.h |  2 +
 2 files changed, 72 insertions(+), 22 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical()
  2018-01-03 11:06 [PATCH v2 0/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
@ 2018-01-03 11:06 ` Geert Uytterhoeven
  2018-01-10 21:14   ` Stephen Boyd
  2018-01-03 11:06 ` [PATCH v2 2/4] clk: Use octal instead of symbolic permissions Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-03 11:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-kernel, Geert Uytterhoeven

The "flags" parameter passed to of_clk_detect_critical() cannot be a
pointer to a real clk_core.flags field, as clk_core is private to the
clock framework internals.

Change the comment to refer to top-level framework flags instead.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Split off from "clk: Fix debugfs_create_*() usage" into a separate
    patch,
  - Change comment to match <linux/clk-provider.h> naming.
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 5ec580914089510a..8c4769c50a71be93 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3927,7 +3927,7 @@ static int parent_ready(struct device_node *np)
  * of_clk_detect_critical() - set CLK_IS_CRITICAL flag from Device Tree
  * @np: Device node pointer associated with clock provider
  * @index: clock index
- * @flags: pointer to clk_core->flags
+ * @flags: pointer to top-level framework flags
  *
  * Detects if the clock-critical property exists and, if so, sets the
  * corresponding CLK_IS_CRITICAL flag.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] clk: Use octal instead of symbolic permissions
  2018-01-03 11:06 [PATCH v2 0/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
  2018-01-03 11:06 ` [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical() Geert Uytterhoeven
@ 2018-01-03 11:06 ` Geert Uytterhoeven
  2018-01-10 21:14   ` Stephen Boyd
  2018-01-03 11:06 ` [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs Geert Uytterhoeven
  2018-01-03 11:06 ` [PATCH v2 4/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
  3 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-03 11:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-kernel, Geert Uytterhoeven

Octal permissions are preferred, as they are easier to read than
symbolic permissions.  Hence replace "S_IRUGO" by "0444".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/clk/clk.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8c4769c50a71be93..240b1d427fadab66 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2600,48 +2600,48 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 
 	core->dentry = d;
 
-	d = debugfs_create_u32("clk_rate", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_rate", 0444, core->dentry,
 			(u32 *)&core->rate);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_accuracy", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_accuracy", 0444, core->dentry,
 			(u32 *)&core->accuracy);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_phase", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_phase", 0444, core->dentry,
 			(u32 *)&core->phase);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_x32("clk_flags", S_IRUGO, core->dentry,
+	d = debugfs_create_x32("clk_flags", 0444, core->dentry,
 			(u32 *)&core->flags);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_prepare_count", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_prepare_count", 0444, core->dentry,
 			(u32 *)&core->prepare_count);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_enable_count", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_enable_count", 0444, core->dentry,
 			(u32 *)&core->enable_count);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_protect_count", 0444, core->dentry,
 			(u32 *)&core->protect_count);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry,
+	d = debugfs_create_u32("clk_notifier_count", 0444, core->dentry,
 			(u32 *)&core->notifier_count);
 	if (!d)
 		goto err_out;
 
 	if (core->num_parents > 1) {
-		d = debugfs_create_file("clk_possible_parents", S_IRUGO,
+		d = debugfs_create_file("clk_possible_parents", 0444,
 				core->dentry, core, &possible_parents_fops);
 		if (!d)
 			goto err_out;
@@ -2737,22 +2737,22 @@ static int __init clk_debug_init(void)
 	if (!rootdir)
 		return -ENOMEM;
 
-	d = debugfs_create_file("clk_summary", S_IRUGO, rootdir, &all_lists,
+	d = debugfs_create_file("clk_summary", 0444, rootdir, &all_lists,
 				&clk_summary_fops);
 	if (!d)
 		return -ENOMEM;
 
-	d = debugfs_create_file("clk_dump", S_IRUGO, rootdir, &all_lists,
+	d = debugfs_create_file("clk_dump", 0444, rootdir, &all_lists,
 				&clk_dump_fops);
 	if (!d)
 		return -ENOMEM;
 
-	d = debugfs_create_file("clk_orphan_summary", S_IRUGO, rootdir,
+	d = debugfs_create_file("clk_orphan_summary", 0444, rootdir,
 				&orphan_list, &clk_summary_fops);
 	if (!d)
 		return -ENOMEM;
 
-	d = debugfs_create_file("clk_orphan_dump", S_IRUGO, rootdir,
+	d = debugfs_create_file("clk_orphan_dump", 0444, rootdir,
 				&orphan_list, &clk_dump_fops);
 	if (!d)
 		return -ENOMEM;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs
  2018-01-03 11:06 [PATCH v2 0/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
  2018-01-03 11:06 ` [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical() Geert Uytterhoeven
  2018-01-03 11:06 ` [PATCH v2 2/4] clk: Use octal instead of symbolic permissions Geert Uytterhoeven
@ 2018-01-03 11:06 ` Geert Uytterhoeven
  2018-01-10  2:02   ` Stephen Boyd
  2018-01-10 21:14   ` Stephen Boyd
  2018-01-03 11:06 ` [PATCH v2 4/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
  3 siblings, 2 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-03 11:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-kernel, Geert Uytterhoeven

Currently the virtual "clk_flags" file in debugfs shows the numeric
value of the top-level framework flags for the specified clock.
Hence the user must manually interpret these values.

Moreover, on big-endian 64-bit systems, the wrong half of the value is
shown, due to the cast from "unsigned long *" to "u32 *".

Fix both issues by showing the symbolic flag names instead.
Any non-standard flags are shown as a hex number.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/clk/clk.c            | 54 ++++++++++++++++++++++++++++++++++++++++++--
 include/linux/clk-provider.h |  2 ++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 240b1d427fadab66..7cb5143c654cd78f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2559,6 +2559,56 @@ static const struct file_operations clk_dump_fops = {
 	.release	= single_release,
 };
 
+static const struct {
+	unsigned long flag;
+	const char *name;
+} clk_flags[] = {
+	{ CLK_SET_RATE_GATE,		"CLK_SET_RATE_GATE",		},
+	{ CLK_SET_PARENT_GATE,		"CLK_SET_PARENT_GATE",		},
+	{ CLK_SET_RATE_PARENT,		"CLK_SET_RATE_PARENT",		},
+	{ CLK_IGNORE_UNUSED,		"CLK_IGNORE_UNUSED",		},
+	{ CLK_IS_BASIC,			"CLK_IS_BASIC",			},
+	{ CLK_GET_RATE_NOCACHE,		"CLK_GET_RATE_NOCACHE",		},
+	{ CLK_SET_RATE_NO_REPARENT,	"CLK_SET_RATE_NO_REPARENT",	},
+	{ CLK_GET_ACCURACY_NOCACHE,	"CLK_GET_ACCURACY_NOCACHE",	},
+	{ CLK_RECALC_NEW_RATES,		"CLK_RECALC_NEW_RATES",		},
+	{ CLK_SET_RATE_UNGATE,		"CLK_SET_RATE_UNGATE",		},
+	{ CLK_IS_CRITICAL,		"CLK_IS_CRITICAL",		},
+	{ CLK_OPS_PARENT_ENABLE,	"CLK_OPS_PARENT_ENABLE",	},
+};
+
+static int clk_flags_dump(struct seq_file *s, void *data)
+{
+	struct clk_core *core = s->private;
+	unsigned long flags = core->flags;
+	unsigned int i;
+
+	for (i = 0; flags && i < ARRAY_SIZE(clk_flags); i++) {
+		if (flags & clk_flags[i].flag) {
+			seq_printf(s, "%s\n", clk_flags[i].name);
+			flags &= ~clk_flags[i].flag;
+		}
+	}
+	if (flags) {
+		/* Unknown flags */
+		seq_printf(s, "0x%lx\n", flags);
+	}
+
+	return 0;
+}
+
+static int clk_flags_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, clk_flags_dump, inode->i_private);
+}
+
+static const struct file_operations clk_flags_fops = {
+	.open		= clk_flags_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static int possible_parents_dump(struct seq_file *s, void *data)
 {
 	struct clk_core *core = s->private;
@@ -2615,8 +2665,8 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_x32("clk_flags", 0444, core->dentry,
-			(u32 *)&core->flags);
+	d = debugfs_create_file("clk_flags", 0444, core->dentry, core,
+				&clk_flags_fops);
 	if (!d)
 		goto err_out;
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 411db2423bd45818..73add58e7d666083 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -20,6 +20,8 @@
  * flags used across common struct clk.  these flags should only affect the
  * top-level framework.  custom flags for dealing with hardware specifics
  * belong in struct clk_foo
+ *
+ * Please update clk_flags[] in drivers/clk/clk.c when making changes here!
  */
 #define CLK_SET_RATE_GATE	BIT(0) /* must be gated across rate change */
 #define CLK_SET_PARENT_GATE	BIT(1) /* must be gated across re-parent */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] clk: Fix debugfs_create_*() usage
  2018-01-03 11:06 [PATCH v2 0/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2018-01-03 11:06 ` [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs Geert Uytterhoeven
@ 2018-01-03 11:06 ` Geert Uytterhoeven
  2018-01-10 21:14   ` Stephen Boyd
  3 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-03 11:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-kernel, Geert Uytterhoeven

When exposing data access through debugfs, the correct
debugfs_create_*() functions must be used, matching the data types.

Remove all casts from data pointers passed to debugfs_create_*()
functions, as such casts prevent the compiler from flagging bugs.

clk_core.rate and .accuracy are "unsigned long", hence casting their
addresses to "u32 *" exposed the wrong halves on big-endian 64-bit
systems.  Fix this by using debugfs_create_ulong() instead.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Fix .flags differently in a separate patch,
  - Split off of_clk_detect_critical() comment update into a separate
    patch.
---
 drivers/clk/clk.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7cb5143c654cd78f..972dd05dbf5558b5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2650,18 +2650,16 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 
 	core->dentry = d;
 
-	d = debugfs_create_u32("clk_rate", 0444, core->dentry,
-			(u32 *)&core->rate);
+	d = debugfs_create_ulong("clk_rate", 0444, core->dentry, &core->rate);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_accuracy", 0444, core->dentry,
-			(u32 *)&core->accuracy);
+	d = debugfs_create_ulong("clk_accuracy", 0444, core->dentry,
+				 &core->accuracy);
 	if (!d)
 		goto err_out;
 
-	d = debugfs_create_u32("clk_phase", 0444, core->dentry,
-			(u32 *)&core->phase);
+	d = debugfs_create_u32("clk_phase", 0444, core->dentry, &core->phase);
 	if (!d)
 		goto err_out;
 
@@ -2671,22 +2669,22 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 		goto err_out;
 
 	d = debugfs_create_u32("clk_prepare_count", 0444, core->dentry,
-			(u32 *)&core->prepare_count);
+			       &core->prepare_count);
 	if (!d)
 		goto err_out;
 
 	d = debugfs_create_u32("clk_enable_count", 0444, core->dentry,
-			(u32 *)&core->enable_count);
+			       &core->enable_count);
 	if (!d)
 		goto err_out;
 
 	d = debugfs_create_u32("clk_protect_count", 0444, core->dentry,
-			(u32 *)&core->protect_count);
+			       &core->protect_count);
 	if (!d)
 		goto err_out;
 
 	d = debugfs_create_u32("clk_notifier_count", 0444, core->dentry,
-			(u32 *)&core->notifier_count);
+			       &core->notifier_count);
 	if (!d)
 		goto err_out;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs
  2018-01-03 11:06 ` [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs Geert Uytterhoeven
@ 2018-01-10  2:02   ` Stephen Boyd
  2018-01-10  7:53     ` Geert Uytterhoeven
  2018-01-10 21:14   ` Stephen Boyd
  1 sibling, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2018-01-10  2:02 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Turquette, linux-clk, linux-kernel

On 01/03, Geert Uytterhoeven wrote:
> Currently the virtual "clk_flags" file in debugfs shows the numeric
> value of the top-level framework flags for the specified clock.
> Hence the user must manually interpret these values.
> 
> Moreover, on big-endian 64-bit systems, the wrong half of the value is
> shown, due to the cast from "unsigned long *" to "u32 *".
> 
> Fix both issues by showing the symbolic flag names instead.
> Any non-standard flags are shown as a hex number.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - New.
> ---
>  drivers/clk/clk.c            | 54 ++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/clk-provider.h |  2 ++
>  2 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 240b1d427fadab66..7cb5143c654cd78f 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2559,6 +2559,56 @@ static const struct file_operations clk_dump_fops = {
>  	.release	= single_release,
>  };
>  
> +static const struct {
> +	unsigned long flag;
> +	const char *name;
> +} clk_flags[] = {
> +	{ CLK_SET_RATE_GATE,		"CLK_SET_RATE_GATE",		},
> +	{ CLK_SET_PARENT_GATE,		"CLK_SET_PARENT_GATE",		},
> +	{ CLK_SET_RATE_PARENT,		"CLK_SET_RATE_PARENT",		},
> +	{ CLK_IGNORE_UNUSED,		"CLK_IGNORE_UNUSED",		},
> +	{ CLK_IS_BASIC,			"CLK_IS_BASIC",			},
> +	{ CLK_GET_RATE_NOCACHE,		"CLK_GET_RATE_NOCACHE",		},
> +	{ CLK_SET_RATE_NO_REPARENT,	"CLK_SET_RATE_NO_REPARENT",	},
> +	{ CLK_GET_ACCURACY_NOCACHE,	"CLK_GET_ACCURACY_NOCACHE",	},
> +	{ CLK_RECALC_NEW_RATES,		"CLK_RECALC_NEW_RATES",		},
> +	{ CLK_SET_RATE_UNGATE,		"CLK_SET_RATE_UNGATE",		},
> +	{ CLK_IS_CRITICAL,		"CLK_IS_CRITICAL",		},
> +	{ CLK_OPS_PARENT_ENABLE,	"CLK_OPS_PARENT_ENABLE",	},
> +};

Thanks!

I wonder if it can be a little simpler with something like the
below patch squashed in? It would also be nice to detect if we
fail to add another flag, but that may mean we need to make the
flags into some sort of enum that we also set equal to BIT(x) and
then have a case statement in the for loop instead of an array
lookup. Not sure that's a big win.

---8<---
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e1fcef8614d5..c8ea2dd32251 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,6 +24,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/sched.h>
 #include <linux/clkdev.h>
+#include <linux/stringify.h>
 
 #include "clk.h"
 
@@ -2558,18 +2559,20 @@ static const struct {
 	unsigned long flag;
 	const char *name;
 } clk_flags[] = {
-	{ CLK_SET_RATE_GATE,		"CLK_SET_RATE_GATE",		},
-	{ CLK_SET_PARENT_GATE,		"CLK_SET_PARENT_GATE",		},
-	{ CLK_SET_RATE_PARENT,		"CLK_SET_RATE_PARENT",		},
-	{ CLK_IGNORE_UNUSED,		"CLK_IGNORE_UNUSED",		},
-	{ CLK_IS_BASIC,			"CLK_IS_BASIC",			},
-	{ CLK_GET_RATE_NOCACHE,		"CLK_GET_RATE_NOCACHE",		},
-	{ CLK_SET_RATE_NO_REPARENT,	"CLK_SET_RATE_NO_REPARENT",	},
-	{ CLK_GET_ACCURACY_NOCACHE,	"CLK_GET_ACCURACY_NOCACHE",	},
-	{ CLK_RECALC_NEW_RATES,		"CLK_RECALC_NEW_RATES",		},
-	{ CLK_SET_RATE_UNGATE,		"CLK_SET_RATE_UNGATE",		},
-	{ CLK_IS_CRITICAL,		"CLK_IS_CRITICAL",		},
-	{ CLK_OPS_PARENT_ENABLE,	"CLK_OPS_PARENT_ENABLE",	},
+#define ENTRY(f) { f, __stringify(f) }
+	ENTRY(CLK_SET_RATE_GATE),
+	ENTRY(CLK_SET_PARENT_GATE),
+	ENTRY(CLK_SET_RATE_PARENT),
+	ENTRY(CLK_IGNORE_UNUSED),
+	ENTRY(CLK_IS_BASIC),
+	ENTRY(CLK_GET_RATE_NOCACHE),
+	ENTRY(CLK_SET_RATE_NO_REPARENT),
+	ENTRY(CLK_GET_ACCURACY_NOCACHE),
+	ENTRY(CLK_RECALC_NEW_RATES),
+	ENTRY(CLK_SET_RATE_UNGATE),
+	ENTRY(CLK_IS_CRITICAL),
+	ENTRY(CLK_OPS_PARENT_ENABLE),
+#undef ENTRY
 };
 
 static int clk_flags_dump(struct seq_file *s, void *data)


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs
  2018-01-10  2:02   ` Stephen Boyd
@ 2018-01-10  7:53     ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-01-10  7:53 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk,
	Linux Kernel Mailing List

Hi Stephen,

On Wed, Jan 10, 2018 at 3:02 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 01/03, Geert Uytterhoeven wrote:
>> Currently the virtual "clk_flags" file in debugfs shows the numeric
>> value of the top-level framework flags for the specified clock.
>> Hence the user must manually interpret these values.
>>
>> Moreover, on big-endian 64-bit systems, the wrong half of the value is
>> shown, due to the cast from "unsigned long *" to "u32 *".
>>
>> Fix both issues by showing the symbolic flag names instead.
>> Any non-standard flags are shown as a hex number.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> I wonder if it can be a little simpler with something like the
> below patch squashed in? It would also be nice to detect if we

Sure, I didn't bother adding a macro for that as the list isn't that long,
and fairly static.

But feel free to fold in that part.

> fail to add another flag, but that may mean we need to make the
> flags into some sort of enum that we also set equal to BIT(x) and
> then have a case statement in the for loop instead of an array
> lookup. Not sure that's a big win.

Yes, you need an enum for that.
The case statement and for loop can be avoided by indexing the array
by enum value.

And the check for unknown flags can be moved to clock registration time
later, if you deem that's the way forward.

Thanks!

> @@ -2558,18 +2559,20 @@ static const struct {
>         unsigned long flag;
>         const char *name;
>  } clk_flags[] = {
> -       { CLK_SET_RATE_GATE,            "CLK_SET_RATE_GATE",            },
> -       { CLK_SET_PARENT_GATE,          "CLK_SET_PARENT_GATE",          },
> -       { CLK_SET_RATE_PARENT,          "CLK_SET_RATE_PARENT",          },
> -       { CLK_IGNORE_UNUSED,            "CLK_IGNORE_UNUSED",            },
> -       { CLK_IS_BASIC,                 "CLK_IS_BASIC",                 },
> -       { CLK_GET_RATE_NOCACHE,         "CLK_GET_RATE_NOCACHE",         },
> -       { CLK_SET_RATE_NO_REPARENT,     "CLK_SET_RATE_NO_REPARENT",     },
> -       { CLK_GET_ACCURACY_NOCACHE,     "CLK_GET_ACCURACY_NOCACHE",     },
> -       { CLK_RECALC_NEW_RATES,         "CLK_RECALC_NEW_RATES",         },
> -       { CLK_SET_RATE_UNGATE,          "CLK_SET_RATE_UNGATE",          },
> -       { CLK_IS_CRITICAL,              "CLK_IS_CRITICAL",              },
> -       { CLK_OPS_PARENT_ENABLE,        "CLK_OPS_PARENT_ENABLE",        },
> +#define ENTRY(f) { f, __stringify(f) }
> +       ENTRY(CLK_SET_RATE_GATE),
> +       ENTRY(CLK_SET_PARENT_GATE),
> +       ENTRY(CLK_SET_RATE_PARENT),
> +       ENTRY(CLK_IGNORE_UNUSED),
> +       ENTRY(CLK_IS_BASIC),
> +       ENTRY(CLK_GET_RATE_NOCACHE),
> +       ENTRY(CLK_SET_RATE_NO_REPARENT),
> +       ENTRY(CLK_GET_ACCURACY_NOCACHE),
> +       ENTRY(CLK_RECALC_NEW_RATES),
> +       ENTRY(CLK_SET_RATE_UNGATE),
> +       ENTRY(CLK_IS_CRITICAL),
> +       ENTRY(CLK_OPS_PARENT_ENABLE),
> +#undef ENTRY
>  };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical()
  2018-01-03 11:06 ` [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical() Geert Uytterhoeven
@ 2018-01-10 21:14   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2018-01-10 21:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Turquette, linux-clk, linux-kernel

On 01/03, Geert Uytterhoeven wrote:
> The "flags" parameter passed to of_clk_detect_critical() cannot be a
> pointer to a real clk_core.flags field, as clk_core is private to the
> clock framework internals.
> 
> Change the comment to refer to top-level framework flags instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] clk: Use octal instead of symbolic permissions
  2018-01-03 11:06 ` [PATCH v2 2/4] clk: Use octal instead of symbolic permissions Geert Uytterhoeven
@ 2018-01-10 21:14   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2018-01-10 21:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Turquette, linux-clk, linux-kernel

On 01/03, Geert Uytterhoeven wrote:
> Octal permissions are preferred, as they are easier to read than
> symbolic permissions.  Hence replace "S_IRUGO" by "0444".
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] clk: Fix debugfs_create_*() usage
  2018-01-03 11:06 ` [PATCH v2 4/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
@ 2018-01-10 21:14   ` Stephen Boyd
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2018-01-10 21:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Turquette, linux-clk, linux-kernel

On 01/03, Geert Uytterhoeven wrote:
> When exposing data access through debugfs, the correct
> debugfs_create_*() functions must be used, matching the data types.
> 
> Remove all casts from data pointers passed to debugfs_create_*()
> functions, as such casts prevent the compiler from flagging bugs.
> 
> clk_core.rate and .accuracy are "unsigned long", hence casting their
> addresses to "u32 *" exposed the wrong halves on big-endian 64-bit
> systems.  Fix this by using debugfs_create_ulong() instead.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs
  2018-01-03 11:06 ` [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs Geert Uytterhoeven
  2018-01-10  2:02   ` Stephen Boyd
@ 2018-01-10 21:14   ` Stephen Boyd
  1 sibling, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2018-01-10 21:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Turquette, linux-clk, linux-kernel

On 01/03, Geert Uytterhoeven wrote:
> Currently the virtual "clk_flags" file in debugfs shows the numeric
> value of the top-level framework flags for the specified clock.
> Hence the user must manually interpret these values.
> 
> Moreover, on big-endian 64-bit systems, the wrong half of the value is
> shown, due to the cast from "unsigned long *" to "u32 *".
> 
> Fix both issues by showing the symbolic flag names instead.
> Any non-standard flags are shown as a hex number.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-10 21:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-03 11:06 [PATCH v2 0/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
2018-01-03 11:06 ` [PATCH v2 1/4] clk: Improve flags doc for of_clk_detect_critical() Geert Uytterhoeven
2018-01-10 21:14   ` Stephen Boyd
2018-01-03 11:06 ` [PATCH v2 2/4] clk: Use octal instead of symbolic permissions Geert Uytterhoeven
2018-01-10 21:14   ` Stephen Boyd
2018-01-03 11:06 ` [PATCH v2 3/4] clk: Show symbolic clock flags in debugfs Geert Uytterhoeven
2018-01-10  2:02   ` Stephen Boyd
2018-01-10  7:53     ` Geert Uytterhoeven
2018-01-10 21:14   ` Stephen Boyd
2018-01-03 11:06 ` [PATCH v2 4/4] clk: Fix debugfs_create_*() usage Geert Uytterhoeven
2018-01-10 21:14   ` Stephen Boyd

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.