[PATCH] scripts/gdb: Fix invocation when CONFIG_COMMON_CLK is not set

[PATCH] scripts/gdb: Fix invocation when CONFIG_COMMON_CLK is not set

[Fabiano Rosas]

CLK_GET_RATE_NOCACHE depends on CONFIG_COMMON_CLK. Importing
constants.py when CONFIG_COMMON_CLK is not defined causes:

  (gdb) lx-symbols
  (...)
    File "scripts/gdb/linux/proc.py", line 15, in <module>
      from linux import constants
    File "scripts/gdb/linux/constants.py", line 2, in <module>
      LX_CLK_GET_RATE_NOCACHE = gdb.parse_and_eval("CLK_GET_RATE_NOCACHE")
  gdb.error: No symbol "CLK_GET_RATE_NOCACHE" in current context.

Fixes: e7e6f462c1be ("scripts/gdb: print cached rate in lx-clk-summary")
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 scripts/gdb/linux/constants.py.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
index 1d73083da6cb..2efbec6b6b8d 100644
--- a/scripts/gdb/linux/constants.py.in
+++ b/scripts/gdb/linux/constants.py.in
@@ -40,7 +40,8 @@
 import gdb
 
 /* linux/clk-provider.h */
-LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
+if IS_BUILTIN(CONFIG_COMMON_CLK):
+    LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
 
 /* linux/fs.h */
 LX_VALUE(SB_RDONLY)
-- 
2.20.1

Re: [PATCH] scripts/gdb: Fix invocation when CONFIG_COMMON_CLK is not set

[Stephen Boyd]

Quoting Fabiano Rosas (2019-05-23 12:53:11)
> diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
> index 1d73083da6cb..2efbec6b6b8d 100644
> --- a/scripts/gdb/linux/constants.py.in
> +++ b/scripts/gdb/linux/constants.py.in
> @@ -40,7 +40,8 @@
>  import gdb
>  
>  /* linux/clk-provider.h */
> -LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
> +if IS_BUILTIN(CONFIG_COMMON_CLK):
> +    LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
>  

Why is this LX_GDBPARSED() instead of LX_VALUE()? From what I can tell
it doesn't need to be runtime evaluated, just assigned to something that
is macro expanded by CPP.

We should probably change the clk-provider.h header to export more
things than what it's exporting right now when COMMON_CLK is disabled.
I'm not sure why the whole thing is wrapped in a big ifdef.

Either way

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH] scripts/gdb: Fix invocation when CONFIG_COMMON_CLK is not set

[Leonard Crestez]

On 5/24/2019 1:46 AM, Stephen Boyd wrote:
> Quoting Fabiano Rosas (2019-05-23 12:53:11)
>> diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
>> index 1d73083da6cb..2efbec6b6b8d 100644
>> --- a/scripts/gdb/linux/constants.py.in
>> +++ b/scripts/gdb/linux/constants.py.in
>> @@ -40,7 +40,8 @@
>>   import gdb
>>   
>>   /* linux/clk-provider.h */
>> -LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
>> +if IS_BUILTIN(CONFIG_COMMON_CLK):
>> +    LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
>>   
> 
> Why is this LX_GDBPARSED() instead of LX_VALUE()? From what I can tell
> it doesn't need to be runtime evaluated, just assigned to something that
> is macro expanded by CPP.

Because CLK_GET_RATE_NOCACHE expands to BIT() which expands to a 
constant with an UL suffix which python doesn't understand.

Alternatively we could redefine the BIT macros inside constants.py.in 
but using gdb features seemed better. We could even try to strip integer 
literal suffixes with sed.

Mentioned before: https://lkml.org/lkml/2019/5/3/341

--
Regards,
Leonard

Re: [PATCH] scripts/gdb: Fix invocation when CONFIG_COMMON_CLK is not set

[Stephen Boyd]

Quoting Leonard Crestez (2019-05-23 16:09:18)
> On 5/24/2019 1:46 AM, Stephen Boyd wrote:
> > Quoting Fabiano Rosas (2019-05-23 12:53:11)
> >> diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
> >> index 1d73083da6cb..2efbec6b6b8d 100644
> >> --- a/scripts/gdb/linux/constants.py.in
> >> +++ b/scripts/gdb/linux/constants.py.in
> >> @@ -40,7 +40,8 @@
> >>   import gdb
> >>   
> >>   /* linux/clk-provider.h */
> >> -LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
> >> +if IS_BUILTIN(CONFIG_COMMON_CLK):
> >> +    LX_GDBPARSED(CLK_GET_RATE_NOCACHE)
> >>   
> > 
> > Why is this LX_GDBPARSED() instead of LX_VALUE()? From what I can tell
> > it doesn't need to be runtime evaluated, just assigned to something that
> > is macro expanded by CPP.
> 
> Because CLK_GET_RATE_NOCACHE expands to BIT() which expands to a 
> constant with an UL suffix which python doesn't understand.
> 
> Alternatively we could redefine the BIT macros inside constants.py.in 
> but using gdb features seemed better. We could even try to strip integer 
> literal suffixes with sed.
> 
> Mentioned before: https://lkml.org/lkml/2019/5/3/341
> 

Ah ok. A comment in the code would have helped me, but o well.

I'd still like to apply this change to clk tree so that we can avoid
needing to do the IS_BUILTIN check entirely.

----8<----
From: Stephen Boyd <sboyd@kernel.org>
Date: Thu, 23 May 2019 17:05:59 -0700
Subject: [PATCH] clk: Remove ifdef for COMMON_CLK in clk-provider.h

This ifdef has been there since the beginning of this file, but it
doesn't really seem to serve any purpose besides obfuscating the struct
definitions and #defines here from compilation units that include it.
Let's always expose these function prototypes and struct definitions so
that code can inspect clk providers without needing to have
CONFIG_COMMON_CLK enabled.

Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 include/linux/clk-provider.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bb6118f79784..3bced2ec9f26 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -9,8 +9,6 @@
 #include <linux/of.h>
 #include <linux/of_clk.h>
 
-#ifdef CONFIG_COMMON_CLK
-
 /*
  * flags used across common struct clk.  these flags should only affect the
  * top-level framework.  custom flags for dealing with hardware specifics
@@ -1019,5 +1017,4 @@ static inline int of_clk_detect_critical(struct device_node *np, int index,
 
 void clk_gate_restore_context(struct clk_hw *hw);
 
-#endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */
-- 
Sent by a computer through tubes