All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
	gregkh@linuxfoundation.org, realwakka@gmail.com,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: pi433: remove need to recompile code to debug fifo content
Date: Tue, 8 Feb 2022 16:54:02 +1300	[thread overview]
Message-ID: <YgHpWpivQHcS+Eh1@mail.google.com> (raw)
In-Reply-To: <a630d8381cee0f543e0d77614052e1d04ab162a5.camel@perches.com>

On Mon, Feb 07, 2022 at 02:15:47AM -0800, Joe Perches wrote:
> > > -#ifdef DEBUG_FIFO_ACCESS
> > > +	/* print content read from fifo for debugging purposes */
> > >  	for (i = 0; i < size; i++)
> > >  		dev_dbg(&spi->dev, "%d - 0x%x\n", i, local_buffer[i + 1]);
> > > -#endif
> 
> If you use
> 
> 	print_hex_dump_debug
> 
> perhaps the DEBUG_FIFO_ACCESS could be removed.
> 

Hi Joe, thanks for taking the time to review my patch.

print_hex_dump_debug is pretty convenient and straight to the point
which I do like. The only thing that I "didn't like" is the fact that 
when configuring dynamic debugging, print_hex_dump_debug only cares 
about 'p' flag and ignores all of the other flags that can be helpful
for tracking things down. 

So essentially I lose track of which function and device instance 
the message belongs to (users can have more than 1 at the same time).

But, because of your suggestion, I came across hex_dump_to_buffer()
which bridged the gap. thanks a lot :)

Would you be comfortable with the following implementation? (Dan, feel
free to chip in)

	char linebuf[FIFO_SIZE * 3] = {0};
	hex_dump_to_buffer(local_buffer + 1, size, 16, 1, linebuf,
			   ARRAY_SIZE(linebuf), false);
	dev_dbg(&spi->dev, "%s\n", linebuf);

thanks,

Paulo Almeida


  reply	other threads:[~2022-02-08  3:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07  4:45 [PATCH] staging: pi433: remove need to recompile code to debug fifo content Paulo Miguel Almeida
2022-02-07 10:06 ` Dan Carpenter
2022-02-07 10:15   ` Joe Perches
2022-02-08  3:54     ` Paulo Miguel Almeida [this message]
2022-02-08  3:56   ` Paulo Miguel Almeida

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=YgHpWpivQHcS+Eh1@mail.google.com \
    --to=paulo.miguel.almeida.rodenas@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=realwakka@gmail.com \
    /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.