From: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
To: gregkh@linuxfoundation.org,
paulo.miguel.almeida.rodenas@gmail.com, realwakka@gmail.com
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH] staging: pi433: remove need to recompile code to debug fifo content
Date: Mon, 7 Feb 2022 17:45:12 +1300 [thread overview]
Message-ID: <YgCj2P59AbFFmnbA@mail.google.com> (raw)
Debugging content present in the FIFO register is tricky as when we read
the FIFO register that changes the content of fifo struct which reduces
number of possible ways of debugging it. Rf69 uC has the possibility of
triggering certain IRQs depending on how many items are in the FIFO
queue, so being able to know what's in there is an important way to
troubleshoot certain problems.
This patch removes the requirement of having to compile pi433 driver
with DEBUG_FIFO_ACCESS set and let that be driven by printk verbositity
level and/or dynamic debug config instead.
Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
Meta-comments:
#1
In my mind, I didn't like the idea of having to change the code and then
echo "module pi433 +p" > <debugfs>/dynamic_debug/control to only then
be able to read stuff being sent/retrieved from fifo. It felt somewhat
redundant at a certain level. On the other hand, I understand that
removing the conditional compilation will force a for-loop to iterate
for no real reason most of the time (max 66 iterations)... so I made a
trade-off and in case anyone disagrees with that, just let me know and I
will be happy to change to a different approach.
#2
In the past, it's been pointed out to me during code review that I tend
to add code comments which could be omitted. In this case, the for-loop
seemed a bit odd without explaining why it's in there. Let me know if
you think I should keep/remove it.
Patch dependency:
This patch depends on the following patches being applied first:
- https://lore.kernel.org/lkml/Yf9ivRB5qpmA5rY2@mail.google.com/
- https://lore.kernel.org/lkml/YgA4XHU4uf6YkOk5@mail.google.com/
---
drivers/staging/pi433/rf69.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index a8def70808d6..901f8db3e3ce 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -6,11 +6,6 @@
* Marcus Wolf <linux@wolf-entwicklungen.de>
*/
-/* enable prosa debug info */
-#undef DEBUG
-/* enable print of values on fifo access */
-#undef DEBUG_FIFO_ACCESS
-
#include <linux/types.h>
#include <linux/spi/spi.h>
@@ -829,9 +824,7 @@ int rf69_set_dagc(struct spi_device *spi, enum dagc dagc)
int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
{
-#ifdef DEBUG_FIFO_ACCESS
int i;
-#endif
struct spi_transfer transfer;
u8 local_buffer[FIFO_SIZE + 1];
int retval;
@@ -851,10 +844,9 @@ int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
retval = spi_sync_transfer(spi, &transfer, 1);
-#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
memcpy(buffer, &local_buffer[1], size);
@@ -863,9 +855,7 @@ int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
int rf69_write_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
{
-#ifdef DEBUG_FIFO_ACCESS
int i;
-#endif
u8 local_buffer[FIFO_SIZE + 1];
if (size > FIFO_SIZE) {
@@ -877,10 +867,9 @@ int rf69_write_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
local_buffer[0] = REG_FIFO | WRITE_BIT;
memcpy(&local_buffer[1], buffer, size);
-#ifdef DEBUG_FIFO_ACCESS
+ /* print content written from fifo for debugging purposes */
for (i = 0; i < size; i++)
dev_dbg(&spi->dev, "0x%x\n", buffer[i]);
-#endif
return spi_write(spi, local_buffer, size + 1);
}
--
2.34.1
next reply other threads:[~2022-02-07 4:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-07 4:45 Paulo Miguel Almeida [this message]
2022-02-07 10:06 ` [PATCH] staging: pi433: remove need to recompile code to debug fifo content Dan Carpenter
2022-02-07 10:15 ` Joe Perches
2022-02-08 3:54 ` Paulo Miguel Almeida
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=YgCj2P59AbFFmnbA@mail.google.com \
--to=paulo.miguel.almeida.rodenas@gmail.com \
--cc=gregkh@linuxfoundation.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).