[U-Boot] [PATCH] lib: Add hexdump

[Stefan Roese]

Hi Alexey,

On 10.06.2016 12:30, Alexey Brodkin wrote:
> Often during debugging session it's very interesting to see
> what data we were dealing with. For example what we write or read
> to/from memory or peripherals.
>
> This change introduces functions that allow to dump binary
> data with one simple function invocation like:
> ------------------->8----------------
> print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
> ------------------->8----------------
>
> which gives us the following:
> ------------------->8----------------
> 00000000: db bc 4d 66 62 61 75 64 72 61 74 65 3d 31 31 35 ..Mfbaudrate=115
> 00000010: 32 30 30 00 62 6f 6f 74 63 6d 64 3d 66 61 74 6c 200.bootcmd=fatl
> 00000020: 73 6f 6c 65 3d 74 74 79 30 3b 20 69 6d 69 00 62 sole=tty0;imi.b
> 00000030: 30 6e 38 00 62 6f 6f 74 3d 32 00 62 6f 6f 74 66 0n8.boot=2.bootf
> 00000040: 62 6f 6f 74 66 69 6c 65 67 65 00 65 74 68 61 63 bootfilege.ethac
> 00000050: 66 64 74 63 6f 6e 74 72 65 74 40 65 30 30 31 38 fdtcontret at e0018
> ------------------->8----------------
>
> Source of hexdump.c was copied from Linux kernel v4.7-rc2.
>
> Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>

Thanks. I often added some debugging code to get a similar feature
while debugging some issues. So this patch is very welcome. One
small comment though below.

> ---
>   include/common.h       |  57 +++++++++
>   include/linux/compat.h |   1 -
>   lib/Kconfig            |   5 +
>   lib/Makefile           |   1 +
>   lib/hexdump.c          | 309 +++++++++++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 372 insertions(+), 1 deletion(-)
>   create mode 100644 lib/hexdump.c
>
> diff --git a/include/common.h b/include/common.h
> index f9f4605..48886b0 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -118,6 +118,63 @@ typedef volatile unsigned char	vu_char;
>   #define debug(fmt, args...)			\
>   	debug_cond(_DEBUG, fmt, ##args)
>
> +
> +enum {
> +	DUMP_PREFIX_NONE,
> +	DUMP_PREFIX_ADDRESS,
> +	DUMP_PREFIX_OFFSET
> +};
> +
> +#ifdef CONFIG_HEXDUMP
> +extern int hex_to_bin(char ch);
> +extern int hex2bin(u8 *dst, const char *src, size_t count);
> +extern char *bin2hex(char *dst, const void *src, size_t count);
> +extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> +			      int groupsize, char *linebuf, size_t linebuflen,
> +			      bool ascii);
> +extern void print_hex_dump(const char *prefix_str, int prefix_type, int rowsize,
> +			   int groupsize, const void *buf, size_t len,
> +			   bool ascii);
> +extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> +				 const void *buf, size_t len);
> +#else /* !CONFIG_HEXDUMP */
> +#define hex_to_bin(...)			do { } while (0)
> +#define hex2bin(...)			do { } while (0)
> +static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> +			      int groupsize, char *linebuf, size_t linebuflen,
> +			      bool ascii)
> +{
> +	return 0;
> +}
> +static inline void print_hex_dump(const char *level, const char *prefix_str,
> +				  int prefix_type, int rowsize, int groupsize,
> +				  const void *buf, size_t len, bool ascii)
> +{
> +}
> +static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> +					const void *buf, size_t len)
> +{
> +}
> +#endif /* CONFIG_HEXDUMP */
> +
> +#if defined(CONFIG_DYNAMIC_DEBUG)
> +#define print_hex_dump_debug(prefix_str, prefix_type, rowsize,	\
> +			     groupsize, buf, len, ascii)	\
> +	dynamic_hex_dump(prefix_str, prefix_type, rowsize,	\
> +			 groupsize, buf, len, ascii)
> +#elif defined(DEBUG)
> +#define print_hex_dump_debug(prefix_str, prefix_type, rowsize,		\
> +			     groupsize, buf, len, ascii)		\
> +	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize,	\
> +		       groupsize, buf, len, ascii)
> +#else
> +static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
> +					int rowsize, int groupsize,
> +					const void *buf, size_t len, bool ascii)
> +{
> +}
> +#endif
> +

Can't you better move this into a new header instead? Other than this,
please feel free to add my:

Reviewed-by: Stefan Roese <sr at denx.de>

Thanks,
Stefan