[PATCH v1 1/2] hexdump: Introduce debug APIs
debug_hex_dump() and debug_hex_dump_bytes() conditionally print the dump based on DEBUG definition.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- include/hexdump.h | 4 ++++ lib/hexdump.c | 60 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/include/hexdump.h b/include/hexdump.h index f2ca4793d699..390af4c8da1e 100644 --- a/include/hexdump.h +++ b/include/hexdump.h @@ -147,6 +147,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, */ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, int groupsize, const void *buf, size_t len, bool ascii); +int debug_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii);
/** * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params @@ -162,5 +164,7 @@ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, */ void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len); +void debug_hex_dump_bytes(const char *prefix_str, int prefix_type, + const void *buf, size_t len);
#endif /* HEXDUMP_H */ diff --git a/lib/hexdump.c b/lib/hexdump.c index 149c93ead8b8..39c9e86649a0 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -125,8 +125,9 @@ overflow1: return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; }
-int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, - int groupsize, const void *buf, size_t len, bool ascii) +static int do_print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii, + bool dbg) { const u8 *ptr = buf; int i, linelen, remaining = len; @@ -146,15 +147,27 @@ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize,
switch (prefix_type) { case DUMP_PREFIX_ADDRESS: - printf("%s%0*lx: %s\n", prefix_str, - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, - (ulong)map_to_sysmem(ptr) + i, linebuf); + if (dbg) { + debug("%s%0*lx: %s\n", prefix_str, + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, + (ulong)map_to_sysmem(ptr) + i, linebuf); + } else } + printf("%s%0*lx: %s\n", prefix_str, + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, + (ulong)map_to_sysmem(ptr) + i, linebuf); + } break; case DUMP_PREFIX_OFFSET: - printf("%s%.8x: %s\n", prefix_str, i, linebuf); + if (dbg) + debug("%s%.8x: %s\n", prefix_str, i, linebuf); + else + printf("%s%.8x: %s\n", prefix_str, i, linebuf); break; default: - printf("%s%s\n", prefix_str, linebuf); + if (dbg) + debug("%s%s\n", prefix_str, linebuf); + else + printf("%s%s\n", prefix_str, linebuf); break; } if (!IS_ENABLED(CONFIG_SPL_BUILD) && ctrlc()) @@ -164,10 +177,30 @@ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, return 0; }
+int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) +{ + return do_print_hex_dump(prefix_str, prefix_type, rowsize, groupsize, + buf, len, ascii, false); +} + +int debug_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) +{ + return do_print_hex_dump(prefix_str, prefix_type, rowsize, groupsize, + buf, len, ascii, true); +} + void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { - print_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true); + do_print_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true, false); +} + +void debug_hex_dump_bytes(const char *prefix_str, int prefix_type, + const void *buf, size_t len) +{ + do_print_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true, true); } #else /* @@ -180,8 +213,19 @@ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, return -ENOSYS; }
+int debug_hex_dump(const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) +{ + return -ENOSYS; +} + void print_hex_dump_bytes(const char *prefix_str, int prefix_type, const void *buf, size_t len) { } + +void debug_hex_dump_bytes(const char *prefix_str, int prefix_type, + const void *buf, size_t len) +{ +} #endif /* CONFIG_HEXDUMP */
For some devices that have slightly different firmware implementation it's good to see what's going on when communicating with the SCU. Add a few debug messages for that purpose.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- arch/x86/lib/scu.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/x86/lib/scu.c b/arch/x86/lib/scu.c index 90ef239bcd3d..c7274e064647 100644 --- a/arch/x86/lib/scu.c +++ b/arch/x86/lib/scu.c @@ -11,6 +11,7 @@ */ #include <common.h> #include <dm.h> +#include <hexdump.h> #include <regmap.h> #include <syscon.h> #include <asm/cpu.h> @@ -48,6 +49,7 @@ struct scu { */ static void scu_ipc_send_command(struct ipc_regs *regs, u32 cmd) { + debug("%s(): 0x%08x\n", __func__, cmd); writel(cmd, ®s->cmd); }
@@ -92,12 +94,18 @@ static int scu_ipc_cmd(struct ipc_regs *regs, u32 cmd, u32 sub, for (i = 0; i < inlen; i++) writel(*in++, ®s->wbuf[i]);
+ debug_hex_dump("SCU in", DUMP_PREFIX_OFFSET, 16, 4, in, inlen * 4, false); + scu_ipc_send_command(regs, (inlen << 16) | (sub << 12) | cmd); err = scu_ipc_check_status(regs);
if (!err) { + u32 *buf = out; + for (i = 0; i < outlen; i++) *out++ = readl(®s->rbuf[i]); + + debug_hex_dump("SCU out", DUMP_PREFIX_OFFSET, 16, 4, buf, outlen * 4, false); }
return err;
On Tue, 9 Nov 2021 at 05:03, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
For some devices that have slightly different firmware implementation it's good to see what's going on when communicating with the SCU. Add a few debug messages for that purpose.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
arch/x86/lib/scu.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Andy,
On Tue, 9 Nov 2021 at 05:03, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
debug_hex_dump() and debug_hex_dump_bytes() conditionally print the dump based on DEBUG definition.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
include/hexdump.h | 4 ++++ lib/hexdump.c | 60 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 8 deletions(-)
+Heinrich Schuchardt
Is it possible to resolve this in the header file? It is a bit odd to have to add DEBUG to hexdump.c in order to get debug output.
Note also the logging system so you can do
log(LOG_CATEGORY, dbg ? LOGL_DEBUG : LOGL_INFO, "fmt ...", ...)
Regards, Simon
On Sat, Nov 13, 2021 at 11:14:28AM -0700, Simon Glass wrote:
On Tue, 9 Nov 2021 at 05:03, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
debug_hex_dump() and debug_hex_dump_bytes() conditionally print the dump based on DEBUG definition.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
include/hexdump.h | 4 ++++ lib/hexdump.c | 60 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 8 deletions(-)
+Heinrich Schuchardt
Heinrich, it's several months passed. What should we do?
Is it possible to resolve this in the header file? It is a bit odd to have to add DEBUG to hexdump.c in order to get debug output.
Note also the logging system so you can do
log(LOG_CATEGORY, dbg ? LOGL_DEBUG : LOGL_INFO, "fmt ...", ...)
On 3/30/22 11:30, Andy Shevchenko wrote:
On Sat, Nov 13, 2021 at 11:14:28AM -0700, Simon Glass wrote:
On Tue, 9 Nov 2021 at 05:03, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
debug_hex_dump() and debug_hex_dump_bytes() conditionally print the dump based on DEBUG definition.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
include/hexdump.h | 4 ++++ lib/hexdump.c | 60 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 8 deletions(-)
+Heinrich Schuchardt
Heinrich, it's several months passed. What should we do?
The logging system was designed by Simon.
Is it possible to resolve this in the header file? It is a bit odd to have to add DEBUG to hexdump.c in order to get debug output.
Note also the logging system so you can do
log(LOG_CATEGORY, dbg ? LOGL_DEBUG : LOGL_INFO, "fmt ...", ...)
I would do something like below.
Then you can use the log command to switch debug output on and off.
Best regards
Heinrich
--- a/include/hexdump.h +++ b/include/hexdump.h @@ -148,6 +148,19 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, int groupsize, const void *buf, size_t len, bool ascii);
+ +int log_hex_dump(enum log_category_t cat, enum log_level_t level, + const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii); + +#define print_hex_dump(args...) ({ \ + log_hex_dump(LOG_CATEGORY, LOGL_INFO, args); \ + }) + +#define debug_hex_dump(args...) ({ \ + log_hex_dump(LOG_CATEGORY, LOGL_DEBUG, args); \ + }) + /** * print_hex_dump_bytes - shorthand form of print_hex_dump() with default params * @prefix_str: string to prefix each line with; diff --git a/lib/hexdump.c b/lib/hexdump.c index 149c93ead8..eb77a9cf94 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -10,6 +10,7 @@
#include <common.h> #include <hexdump.h> +#include <log.h> #include <mapmem.h> #include <linux/ctype.h> #include <linux/compat.h> @@ -125,8 +126,9 @@ overflow1: return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; }
-int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize, - int groupsize, const void *buf, size_t len, bool ascii) +int log_hex_dump(enum log_category_t cat, enum log_level_t level, + const char *prefix_str, int prefix_type, int rowsize, + int groupsize, const void *buf, size_t len, bool ascii) { const u8 *ptr = buf; int i, linelen, remaining = len; @@ -146,15 +148,17 @@ int print_hex_dump(const char *prefix_str, int prefix_type, int rowsize,
switch (prefix_type) { case DUMP_PREFIX_ADDRESS: - printf("%s%0*lx: %s\n", prefix_str, - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, - (ulong)map_to_sysmem(ptr) + i, linebuf); + log(cat, level, + "%s%0*lx: %s\n", prefix_str, + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, + (ulong)map_to_sysmem(ptr) + i, linebuf); break; case DUMP_PREFIX_OFFSET: - printf("%s%.8x: %s\n", prefix_str, i, linebuf); + log(cat, level, + "%s%.8x: %s\n", prefix_str, i, linebuf); break; default: - printf("%s%s\n", prefix_str, linebuf); + log(cat, level, "%s%s\n", prefix_str, linebuf); break; } if (!IS_ENABLED(CONFIG_SPL_BUILD) && ctrlc())
participants (3)
-
Andy Shevchenko -
Heinrich Schuchardt -
Simon Glass