Re: [PATCH v7 04/10] usb: dbc: add debug buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26.01.2016 14:58, Lu Baolu wrote:
"printk" is not suitable for dbc debugging especially when console
is in usage. This patch adds a debug buffer in dbc driver and puts
the debug messages in this local buffer. The debug buffer could be
dumped whenever the console is not in use. This part of code will
not be visible unless DBC_DEBUG is defined.

Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
  drivers/usb/early/xhci-dbc.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index 41ce116..6855048 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
  static struct xdbc_state *xdbcp = &xdbc_stat;

  #ifdef DBC_DEBUG
-/* place holder */
-#define	xdbc_trace	printk
+#define	XDBC_DEBUG_BUF_SIZE	(PAGE_SIZE * 32)

Does it really need to be this huge? minimum 4096 * 32 ~ 128k
The kernel ring  buffer is about the same size (16k - 256k)

+#define	MSG_MAX_LINE		128

with 128 characters per line this would fit ~1000 lines

+static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
+static void xdbc_trace(const char *fmt, ...)
+{
+	int i, size;
+	va_list args;
+	static int pos;
+	char temp_buf[MSG_MAX_LINE];
+
+	if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+		return;
+
+	memset(temp_buf, 0, MSG_MAX_LINE);
+	va_start(args, fmt);
+	vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
+	va_end(args);
+
+	i = 0;
+	size = strlen(temp_buf);
+	while (i < size) {
+		xdbc_debug_buf[pos] = temp_buf[i];
+		pos++;
+		i++;
+
+		if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+			break;
+	}

how about something like:

size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
memcpy(xdbc_debug_buf + pos, temp_buf, size)
pos += size;

(might need some "-1" and off by one checking..)

+}
+
+static void xdbc_dump_debug_buffer(void)
+{
+	int index = 0;
+	int count = 0;
+	char dump_buf[MSG_MAX_LINE];
+
+	xdbc_trace("The end of DbC trace buffer\n");
+	pr_notice("DBC debug buffer:\n");
+	memset(dump_buf, 0, MSG_MAX_LINE);
+
+	while (index < XDBC_DEBUG_BUF_SIZE) {
+		if (!xdbc_debug_buf[index])
+			break;
+
+		if (xdbc_debug_buf[index] == '\n' ||
+				count >= MSG_MAX_LINE - 1) {
+			pr_notice("DBC: @%08x %s\n", index, dump_buf);

Is showing the he index (position in debug buffer) useful here?

+			memset(dump_buf, 0, MSG_MAX_LINE);
+			count = 0;
+		} else {
+			dump_buf[count] = xdbc_debug_buf[index];
+			count++;
+		}
+
+		index++;
+	}

So we have one huge buffer that xdbc keeps on filling as the initialization progresses.
It is never emptied, or overwritten (circular).
When dumped it always dumps the whole thing, copying one character
at a time.

As this is only used for debugging during xdbc development/debugging, and never enabled
even if xdbc early printk is used, I don't think optimization really matters.

Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver even nearly
writing that much debug data.
-Mathias

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux