Re: [PATCH 2/2] GenWQE: Replace dynamic_hex_dump withprint_hex_dump_debug

From: Greg KH
Date: Fri Dec 20 2013 - 10:55:19 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/