Subject: Re: [PATCH 2/2] GenWQE: Replace dynamic_hex_dump with print_hex_dump_debug

From: Greg KH <[email-masked]>
Date: Fri, 20 Dec 2013 07:55:19 -0800

On Fri, Dec 20, 2013 at 04:49:03PM +0100, Frank Haverkamp wrote:
> Hi Greg,
> 
> Am Freitag, den 20.12.2013, 07:33 -0800 schrieb Greg KH:
> > On Fri, Dec 20, 2013 at 04:26:11PM +0100, Frank Haverkamp wrote:
> > > As requested by Greg, replacing the hexdump function from dynamic_debug.h
> > > with one defined in printk.h. I hope I picked the right one.
> > 
> > No, just use the "%*ph" modifier for printk.
> > 
> > So, you can do it all on just one line:
> >   	scnprintf(prefix, sizeof(prefix), "%s %s: %*ph\n",
> >   		  GENWQE_DEVNAME, pci_name(pci_dev), size, buff);
> 
> Oh, I did not know that such a feature exists. The buffer I like to dump
> is 256 bytes large. Looking in to Documentation/printk-formats.txt it
> says:
> 
> Raw buffer as a hex string:
>         %*ph    00 01 02  ...  3f
>         %*phC   00:01:02: ... :3f
>         %*phD   00-01-02- ... -3f
>         %*phN   000102 ... 3f
> 
>         For printing a small buffers (up to 64 bytes long) as a hex
> string with
>         certain separator. For the larger buffers consider to use
>         print_hex_dump().
> 
> So is my choice still not ok, considering the data size I like to dump?

Ah, I didn't realize it was limited to 64 bytes long.  So nevermind,
your original patch is fine, I'll queue it up.

> > And even then, do you really need this genwqe_hexdump() function at all
> > anymore?  What is it used for?
> 
> We were using this feature to dump work-requests before and after they
> had been sent to the card. This was very helpful when there was a
> problem with the card executing one of those requests. Now since the
> card is really running stable, you  could argue that we do not need it
> anymore. So if you really, really insist, I will remove the feature else
> I would like to keep it.

If you want to keep it, that's fine, just checking.

thanks,

greg k-h