All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jes Sorensen <Jes.Sorensen@redhat.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Joe Perches <joe@perches.com>, Kalle Valo <kvalo@codeaurora.org>,
	linux-wireless@vger.kernel.org,
	Greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v1 1/1] rtl8xxxu: tuse %*ph to dump buffers
Date: Thu, 26 May 2016 16:16:49 -0400	[thread overview]
Message-ID: <wrfjlh2wbmmm.fsf@redhat.com> (raw)
In-Reply-To: <1464281830.27624.27.camel@linux.intel.com> (Andy Shevchenko's message of "Thu, 26 May 2016 19:57:10 +0300")

Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> On Thu, 2016-05-26 at 09:02 -0700, Joe Perches wrote:
>> On Thu, 2016-05-26 at 17:52 +0300, Andy Shevchenko wrote:
>> > Use %*ph specifier to dump small buffers in hex format instead of
>> > doing this
>> > byte-by-byte.
>> []
>> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c
>> > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c
>> []
>> > @@ -413,13 +413,8 @@ static int rtl8192cu_parse_efuse(struct
>> > rtl8xxxu_priv *priv)
>> >  		dev_info(&priv->udev->dev,
>> >  			 "%s: dumping efuse (0x%02zx bytes):\n",
>> >  			 __func__, sizeof(struct rtl8192cu_efuse));
>> > -		for (i = 0; i < sizeof(struct rtl8192cu_efuse); i
>> > += 8) {
>> > -			dev_info(&priv->udev->dev, "%02x: "
>> > -				 "%02x %02x %02x %02x %02x %02x
>> > %02x %02x\n", i,
>> > -				 raw[i], raw[i + 1], raw[i + 2],
>> > -				 raw[i + 3], raw[i + 4], raw[i +
>> > 5],
>> > -				 raw[i + 6], raw[i + 7]);
>> > -		}
>> > +		for (i = 0; i < sizeof(struct rtl8192cu_efuse); i
>> > += 8)
>> > +			dev_info(&priv->udev->dev, "%02x: %8ph\n",
>> > i, &raw[i]);
>> 
>> print_hex_dump_bytes?
>
> I know, perhaps I had to mention that it is not too beneficial here. 
>
>> or maybe add a dev_print_hex_dump variant?
>
> I don't know how much users we have to introduce a such. This is
> actually a main reason I avoid to use print_hex_dump() above. Btw, the
> above might be collapsed into one function instead of 3 times of copy-n-
> paste.

I think a dev_print_hex_dump() would be useful here. When I first read
your patch I was a bit hmmmm but then I realized why you had stuck with
the %8ph rather than print_hex_dump().

I'll apply it shortly.

Jes

      reply	other threads:[~2016-05-26 20:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-26 14:52 [PATCH v1 1/1] rtl8xxxu: tuse %*ph to dump buffers Andy Shevchenko
2016-05-26 16:02 ` Joe Perches
2016-05-26 16:57   ` Andy Shevchenko
2016-05-26 20:16     ` Jes Sorensen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=wrfjlh2wbmmm.fsf@redhat.com \
    --to=jes.sorensen@redhat.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.