[PATCH 0/2] gdb/scripts: Improve lx-clk-summary

[PATCH 0/2] gdb/scripts: Improve lx-clk-summary

[Leonard Crestez]

The earlier series adding clk support to gdb/scripts was quickly
accepted but some concerns were raised by Stephen Boyd so this series
attempts to address them.

Link to previous series: https://lkml.org/lkml/2019/4/22/55

This is not a v2 and squashing is not expected.

Fields other than clk rate not covered because they're much more rarely
used and cache logic can get more complicated and brittle.

LX_GDBPARSED is used in constants.py.in because python does not
understand C integer literal suffixes like the "1UL" from the definition
of BIT() used by CLK_GET_RATE_NOCACHE. Alternative workarounds would be
hacking away UL suffixes with sed or redefining BIT&co but relying on
gdb evaluation is easier and much more flexible.

Leonard Crestez (2):
  scripts/gdb: Cleanup error handling in list helpers
  scripts/gdb: Print cached rate in lx-clk-summary

 scripts/gdb/linux/clk.py          | 21 ++++++++++++++-------
 scripts/gdb/linux/constants.py.in |  4 ++++
 scripts/gdb/linux/lists.py        | 10 ++--------
 3 files changed, 20 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH 1/2] scripts/gdb: Cleanup error handling in list helpers

[Leonard Crestez]

An incorrect argument to list_for_each is an internal error in gdb
scripts so a TypeError should be raised. The gdb.GdbError exception type
is intended for user errors such as incorrect invocation.

Drop the type assertion in list_for_each_entry because list_for_each isn't
going to suddenly yield something else.

Applies to both list and hlist

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 scripts/gdb/linux/lists.py | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/scripts/gdb/linux/lists.py b/scripts/gdb/linux/lists.py
index 55356b66f8ea..c487ddf09d38 100644
--- a/scripts/gdb/linux/lists.py
+++ b/scripts/gdb/linux/lists.py
@@ -22,45 +22,39 @@ hlist_node = utils.CachedType("struct hlist_node")
 
 def list_for_each(head):
     if head.type == list_head.get_type().pointer():
         head = head.dereference()
     elif head.type != list_head.get_type():
-        raise gdb.GdbError("Must be struct list_head not {}"
+        raise TypeError("Must be struct list_head not {}"
                            .format(head.type))
 
     node = head['next'].dereference()
     while node.address != head.address:
         yield node.address
         node = node['next'].dereference()
 
 
 def list_for_each_entry(head, gdbtype, member):
     for node in list_for_each(head):
-        if node.type != list_head.get_type().pointer():
-            raise TypeError("Type {} found. Expected struct list_head *."
-                            .format(node.type))
         yield utils.container_of(node, gdbtype, member)
 
 
 def hlist_for_each(head):
     if head.type == hlist_head.get_type().pointer():
         head = head.dereference()
     elif head.type != hlist_head.get_type():
-        raise gdb.GdbError("Must be struct hlist_head not {}"
+        raise TypeError("Must be struct hlist_head not {}"
                            .format(head.type))
 
     node = head['first'].dereference()
     while node.address:
         yield node.address
         node = node['next'].dereference()
 
 
 def hlist_for_each_entry(head, gdbtype, member):
     for node in hlist_for_each(head):
-        if node.type != hlist_node.get_type().pointer():
-            raise TypeError("Type {} found. Expected struct hlist_head *."
-                            .format(node.type))
         yield utils.container_of(node, gdbtype, member)
 
 
 def list_check(head):
     nb = 0
-- 
2.17.1

Re: [PATCH 1/2] scripts/gdb: Cleanup error handling in list helpers

[Stephen Boyd]

Quoting Leonard Crestez (2019-05-03 04:19:31)
> An incorrect argument to list_for_each is an internal error in gdb
> scripts so a TypeError should be raised. The gdb.GdbError exception type
> is intended for user errors such as incorrect invocation.
> 
> Drop the type assertion in list_for_each_entry because list_for_each isn't
> going to suddenly yield something else.
> 
> Applies to both list and hlist

This should be done to other "type errors" in the gdb scripts too.

> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---

Either way,

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

[PATCH 2/2] scripts/gdb: Print cached rate in lx-clk-summary

[Leonard Crestez]

The clk rate is always stored in clk_core but might be out of date and
require calls to update from hardware.

Deal with that case by printing a (c) suffix.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---
 scripts/gdb/linux/clk.py          | 21 ++++++++++++++-------
 scripts/gdb/linux/constants.py.in |  4 ++++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/scripts/gdb/linux/clk.py b/scripts/gdb/linux/clk.py
index 6bf71976b55d..061aecfa294e 100644
--- a/scripts/gdb/linux/clk.py
+++ b/scripts/gdb/linux/clk.py
@@ -3,42 +3,49 @@
 # Copyright (c) NXP 2019
 
 import gdb
 import sys
 
-from linux import utils, lists
+from linux import utils, lists, constants
 
 clk_core_type = utils.CachedType("struct clk_core")
 
 
 def clk_core_for_each_child(hlist_head):
     return lists.hlist_for_each_entry(hlist_head,
             clk_core_type.get_type().pointer(), "child_node")
 
 
 class LxClkSummary(gdb.Command):
-    """Print Linux kernel log buffer."""
+    """Print clk tree summary
+
+Output is a subset of /sys/kernel/debug/clk/clk_summary
+
+No calls are made during printing, instead a (c) if printed after values which
+are cached and potentially out of date"""
 
     def __init__(self):
         super(LxClkSummary, self).__init__("lx-clk-summary", gdb.COMMAND_DATA)
 
     def show_subtree(self, clk, level):
-        gdb.write("%*s%-*s %7d %8d %8d\n" % (
+        gdb.write("%*s%-*s %7d %8d %8d %11lu%s\n" % (
                 level * 3 + 1, "",
                 30 - level * 3,
                 clk['name'].string(),
                 clk['enable_count'],
                 clk['prepare_count'],
-                clk['protect_count']))
+                clk['protect_count'],
+                clk['rate'],
+                '(c)' if clk['flags'] & constants.LX_CLK_GET_RATE_NOCACHE else '   '))
 
         for child in clk_core_for_each_child(clk['children']):
             self.show_subtree(child, level + 1)
 
     def invoke(self, arg, from_tty):
-        gdb.write("                                 enable  prepare  protect\n")
-        gdb.write("   clock                          count    count    count\n")
-        gdb.write("---------------------------------------------------------\n")
+        gdb.write("                                 enable  prepare  protect               \n")
+        gdb.write("   clock                          count    count    count        rate   \n")
+        gdb.write("------------------------------------------------------------------------\n")
         for clk in clk_core_for_each_child(gdb.parse_and_eval("clk_root_list")):
             self.show_subtree(clk, 0)
         for clk in clk_core_for_each_child(gdb.parse_and_eval("clk_orphan_list")):
             self.show_subtree(clk, 0)
 
diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
index 76f46f427b96..1d73083da6cb 100644
--- a/scripts/gdb/linux/constants.py.in
+++ b/scripts/gdb/linux/constants.py.in
@@ -10,10 +10,11 @@
  *
  * This work is licensed under the terms of the GNU GPL version 2.
  *
  */
 
+#include <linux/clk-provider.h>
 #include <linux/fs.h>
 #include <linux/hrtimer.h>
 #include <linux/mount.h>
 #include <linux/of_fdt.h>
 #include <linux/threads.h>
@@ -36,10 +37,13 @@
 /* The build system will take care of deleting everything above this marker */
 <!-- end-c-headers -->
 
 import gdb
 
+/* linux/clk-provider.h */
+LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
+
 /* linux/fs.h */
 LX_VALUE(SB_RDONLY)
 LX_VALUE(SB_SYNCHRONOUS)
 LX_VALUE(SB_MANDLOCK)
 LX_VALUE(SB_DIRSYNC)
-- 
2.17.1