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

[Alexey Brodkin]

Hi Stefan,

On Fri, 2016-06-10 at 12:39 +0200, Stefan Roese wrote:
> 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.

True, I remember spending time googling for good snipped that will
do dumping every time I see data being corrupted here and there.
So finally I took a liberty and copied Linux implementation of the dumper :)

> > 
> > ---
> >   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:

I thought about it but then decided to keep changes in existing header.
Now will move in a separate one.

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

Thanks for that!

-Alexey