[PATCH 0/3] scripts/gdb: Initial clk support

[PATCH 0/3] scripts/gdb: Initial clk support

[Leonard Crestez]

Tested with openocd on imx but should be useful everywhere. It is common
for clk issues to lockup the system and make /sys inaccesible.

Not sure where to send this; there doesn't appear to be any list more
specific than LKML.

Leonard Crestez (3):
  scripts/gdb: Add hlist utilities
  scripts/gdb: Initial clk support: lx-clk-summary
  scripts/gdb: Add $lx_clk_core_lookup function

 scripts/gdb/linux/clk.py   | 69 ++++++++++++++++++++++++++++++++++++++
 scripts/gdb/linux/lists.py | 23 +++++++++++++
 scripts/gdb/vmlinux-gdb.py |  1 +
 3 files changed, 93 insertions(+)
 create mode 100644 scripts/gdb/linux/clk.py

-- 
2.17.1

[PATCH 1/3] scripts/gdb: Add hlist utilities

[Leonard Crestez]

This allows easily examining kernel hlists in python.

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

diff --git a/scripts/gdb/linux/lists.py b/scripts/gdb/linux/lists.py
index 1987d756b36b..55356b66f8ea 100644
--- a/scripts/gdb/linux/lists.py
+++ b/scripts/gdb/linux/lists.py
@@ -14,10 +14,12 @@
 import gdb
 
 from linux import utils
 
 list_head = utils.CachedType("struct list_head")
+hlist_head = utils.CachedType("struct hlist_head")
+hlist_node = utils.CachedType("struct hlist_node")
 
 
 def list_for_each(head):
     if head.type == list_head.get_type().pointer():
         head = head.dereference()
@@ -37,10 +39,31 @@ def list_for_each_entry(head, gdbtype, member):
             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 {}"
+                           .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
     if (head.type == list_head.get_type().pointer()):
         head = head.dereference()
     elif (head.type != list_head.get_type()):
-- 
2.17.1

Re: [PATCH 1/3] scripts/gdb: Add hlist utilities

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-22 01:26:56)
> This allows easily examining kernel hlists in python.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

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

> +
> +
> +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 *."

Maybe drop the full-stop? It looks weird to see struct list_head *.

> +                            .format(node.type))
> +        yield utils.container_of(node, gdbtype, member)
> +
> +

Re: [PATCH 1/3] scripts/gdb: Add hlist utilities

[Leonard Crestez]

On 4/22/2019 11:41 PM, Stephen Boyd wrote:
> Quoting Leonard Crestez (2019-04-22 01:26:56)
>> This allows easily examining kernel hlists in python.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> 
>> +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 *."
> 
> Maybe drop the full-stop? It looks weird to see struct list_head *.

Makes sense, it was copy/pasted from list_for_each_entry. Since this 
iterates over items from hlist_for_each the check is only useful as an
assertion anyway so I'll just drop it.

The hlist_for_each/list_for_each raise gdb.GdbError on type mismatch; as 
far as I understand the difference is that GdbError won't print a python 
backtrace. However these are internal helpers so TypeError makes more sense.

[PATCH 2/3] scripts/gdb: Initial clk support: lx-clk-summary

[Leonard Crestez]

Add an lx-clk-summary command which prints a subset of
/sys/kernel/debug/clk/clk_summary.

This can be used to examine hangs caused by clk not being enabled.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 scripts/gdb/linux/clk.py   | 46 ++++++++++++++++++++++++++++++++++++++
 scripts/gdb/vmlinux-gdb.py |  1 +
 2 files changed, 47 insertions(+)
 create mode 100644 scripts/gdb/linux/clk.py

diff --git a/scripts/gdb/linux/clk.py b/scripts/gdb/linux/clk.py
new file mode 100644
index 000000000000..a9129d865c5d
--- /dev/null
+++ b/scripts/gdb/linux/clk.py
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) NXP 2019
+
+import gdb
+import sys
+
+from linux import utils, lists
+
+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."""
+
+    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" % (
+                level * 3 + 1, "",
+                30 - level * 3,
+                clk['name'].string(),
+                clk['enable_count'],
+                clk['prepare_count'],
+                clk['protect_count']))
+
+        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")
+        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)
+
+
+LxClkSummary()
diff --git a/scripts/gdb/vmlinux-gdb.py b/scripts/gdb/vmlinux-gdb.py
index 033578cc4cd7..eff5a48ac026 100644
--- a/scripts/gdb/vmlinux-gdb.py
+++ b/scripts/gdb/vmlinux-gdb.py
@@ -32,5 +32,6 @@ else:
     import linux.lists
     import linux.rbtree
     import linux.proc
     import linux.constants
     import linux.timerlist
+    import linux.clk
-- 
2.17.1

Re: [PATCH 2/3] scripts/gdb: Initial clk support: lx-clk-summary

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-22 01:26:57)
> +
> +
> +class LxClkSummary(gdb.Command):
> +    """Print Linux kernel log buffer."""

This comment needs an update.

> +
> +    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" % (
> +                level * 3 + 1, "",
> +                30 - level * 3,
> +                clk['name'].string(),
> +                clk['enable_count'],
> +                clk['prepare_count'],
> +                clk['protect_count']))

Would be nice to also print the rate, phase, etc. It's all pretty much
there already so it's just dumping data out. If it's a clk that has
CLK_GET_RATE_NOCACHE then I guess we have to print out '<unknown>'
because it relies on runtime code execution.

> +
> +        for child in clk_core_for_each_child(clk['children']):
> +            self.show_subtree(child, level + 1)

[PATCH 3/3] scripts/gdb: Add $lx_clk_core_lookup function

[Leonard Crestez]

Finding an individual clk_core requires walking the tree which can be
quite complicated so add a helper for easy access.

(gdb) print *(struct clk_scu*)$lx_clk_core_lookup("uart0_clk")->hw

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

diff --git a/scripts/gdb/linux/clk.py b/scripts/gdb/linux/clk.py
index a9129d865c5d..6bf71976b55d 100644
--- a/scripts/gdb/linux/clk.py
+++ b/scripts/gdb/linux/clk.py
@@ -42,5 +42,28 @@ class LxClkSummary(gdb.Command):
         for clk in clk_core_for_each_child(gdb.parse_and_eval("clk_orphan_list")):
             self.show_subtree(clk, 0)
 
 
 LxClkSummary()
+
+
+class LxClkCoreLookup(gdb.Function):
+    """Find struct clk_core by name"""
+
+    def __init__(self):
+        super(LxClkCoreLookup, self).__init__("lx_clk_core_lookup")
+
+    def lookup_hlist(self, hlist_head, name):
+        for child in clk_core_for_each_child(hlist_head):
+            if child['name'].string() == name:
+                return child
+            result = self.lookup_hlist(child['children'], name)
+            if result:
+                return result
+
+    def invoke(self, name):
+        name = name.string()
+        return (self.lookup_hlist(gdb.parse_and_eval("clk_root_list"), name) or
+                self.lookup_hlist(gdb.parse_and_eval("clk_orphan_list"), name))
+
+
+LxClkCoreLookup()
-- 
2.17.1

Re: [PATCH 3/3] scripts/gdb: Add $lx_clk_core_lookup function

[Leonard Crestez]

On 4/22/2019 11:18 PM, Stephen Boyd wrote:
> Quoting Leonard Crestez (2019-04-22 01:26:57)
>> diff --git a/scripts/gdb/linux/clk.py b/scripts/gdb/linux/clk.py
>> +class LxClkCoreLookup(gdb.Function):
>> +    """Find struct clk_core by name"""
>> +
>> +    def __init__(self):
>> +        super(LxClkCoreLookup, self).__init__("lx_clk_core_lookup")
>> +
>> +    def lookup_hlist(self, hlist_head, name):
>> +        for child in clk_core_for_each_child(hlist_head):
>> +            if child['name'].string() == name:
> 
> Do you need to do the .string() for comparison? Or does it work just as
> well to compare a gdb.Value object to a python string? It would be nice
> if the gdb.Value object could figure out that they're not both gdb.Value
> objects so it can do a string comparison itself.

The gdb manual is not clear on how comparisons work on gdb.Value types. 
Converting to a python string and comparing in python work well, using 
== on string gdb.Values results in this:

     gdb.error: evaluation of this expression requires the program to 
have a function "malloc"

My guess is gdb attempts to convert both arguments to gdb.Value and do 
the comparison via a call on the target? This is very undesirable here.

I get the same error if "name" is a gdb.Value instead of being converted 
to a string in invoke().

--
Regards,
Leonard

Re: [PATCH 3/3] scripts/gdb: Add $lx_clk_core_lookup function

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-23 04:12:49)
> On 4/22/2019 11:18 PM, Stephen Boyd wrote:
> > Do you need to do the .string() for comparison? Or does it work just as
> > well to compare a gdb.Value object to a python string? It would be nice
> > if the gdb.Value object could figure out that they're not both gdb.Value
> > objects so it can do a string comparison itself.
> 
> The gdb manual is not clear on how comparisons work on gdb.Value types. 
> Converting to a python string and comparing in python work well, using 
> == on string gdb.Values results in this:
> 
>      gdb.error: evaluation of this expression requires the program to 
> have a function "malloc"
> 
> My guess is gdb attempts to convert both arguments to gdb.Value and do 
> the comparison via a call on the target? This is very undesirable here.
> 
> I get the same error if "name" is a gdb.Value instead of being converted 
> to a string in invoke().
> 

Ok. Thanks for checking. It might be worth filing a bug with gdb to see
if this can be improved in the future.

Re: [PATCH 0/3] scripts/gdb: Initial clk support

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-22 01:26:56)
> Tested with openocd on imx but should be useful everywhere. It is common
> for clk issues to lockup the system and make /sys inaccesible.

Thanks for working on this. I had this on my backlogged todo list. One
minor comment on the clk summary patch but otherwise it looks OK to me
and I'm happy to have Andrew shepherd the patches through.

> 
> Not sure where to send this; there doesn't appear to be any list more
> specific than LKML.

Please Cc linux-clk on clk related patches.