[PATCH 00/17] usb: host: ehci-dbg: cleanup and refactoring

[PATCH 00/17] usb: host: ehci-dbg: cleanup and refactoring

[Geyslan G. Bem]

This patchset removes all errors reported by checkpatch in addition to
some refactoring.

Geyslan G. Bem (17):
  usb: host: ehci-dbg: remove space before open parenthesis
  usb: host: ehci-dbg: remove space before open square bracket
  usb: host: ehci-dbg: use C89-style comments
  usb: host: ehci-dbg: move trailing statements to next line
  usb: host: ehci-dbg: fix up closing parenthesis
  usb: host: ehci-dbg: put spaces around operators
  usb: host: ehci-dbg: fix unsigned comparison
  usb: host: ehci-dbg: remove unnecessary space after cast
  usb: host: ehci-dbg: fix up function definitions
  usb: host: ehci-dbg: use a blank line after struct declarations
  usb: host: ehci-dbg: convert macro to inline function
  usb: host: ehci-dbg: add blank line after declarations
  usb: host: ehci-dbg: remove blank line before close brace
  usb: host: ehci-dbg: replace sizeof operand
  usb: host: ehci-dbg: enclose conditional blocks with braces
  usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size
  usb: host: ehci-dbg: refactor fill_periodic_buffer function

 drivers/usb/host/ehci-dbg.c | 585 ++++++++++++++++++++++++--------------------
 1 file changed, 315 insertions(+), 270 deletions(-)

-- 
2.6.4

[PATCH 01/17] usb: host: ehci-dbg: remove space before open parenthesis

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch. The vast
majority of changes in this patch are removing spaces before opening
parenthesis, but in some cases, a few additional changes are made to fix
other coding style issues.

These additional changes are:

 - Spaces around >> on line 50.
 - On line 55 a call to ehci_dbg reduced to a single line.
 - sizeof operands surrounded with parenthesis on lines 877, 883, 889
   and 901.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    Before this patch there are 105 warnings about spaces before opening
    parenthesis. After there are still 6. These lines concern to macros that
    will be modified to inline functions in sequential patches.
    
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 199 ++++++++++++++++++++++----------------------
 1 file changed, 99 insertions(+), 100 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b7d623f..fcbbdfa 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -24,41 +24,40 @@
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
-static void dbg_hcs_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 {
 	u32	params = ehci_readl(ehci, &ehci->caps->hcs_params);
 
-	ehci_dbg (ehci,
+	ehci_dbg(ehci,
 		"%s hcs_params 0x%x dbg=%d%s cc=%d pcc=%d%s%s ports=%d\n",
 		label, params,
-		HCS_DEBUG_PORT (params),
-		HCS_INDICATOR (params) ? " ind" : "",
-		HCS_N_CC (params),
-		HCS_N_PCC (params),
-		HCS_PORTROUTED (params) ? "" : " ordered",
-		HCS_PPC (params) ? "" : " !ppc",
-		HCS_N_PORTS (params)
+		HCS_DEBUG_PORT(params),
+		HCS_INDICATOR(params) ? " ind" : "",
+		HCS_N_CC(params),
+		HCS_N_PCC(params),
+		HCS_PORTROUTED(params) ? "" : " ordered",
+		HCS_PPC(params) ? "" : " !ppc",
+		HCS_N_PORTS(params)
 		);
 	/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
-	if (HCS_PORTROUTED (params)) {
+	if (HCS_PORTROUTED(params)) {
 		int i;
 		char buf [46], tmp [7], byte;
 
 		buf[0] = 0;
-		for (i = 0; i < HCS_N_PORTS (params); i++) {
+		for (i = 0; i < HCS_N_PORTS(params); i++) {
 			// FIXME MIPS won't readb() ...
-			byte = readb (&ehci->caps->portroute[(i>>1)]);
+			byte = readb(&ehci->caps->portroute[(i >> 1)]);
 			sprintf(tmp, "%d ",
 				((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
 			strcat(buf, tmp);
 		}
-		ehci_dbg (ehci, "%s portroute %s\n",
-				label, buf);
+		ehci_dbg(ehci, "%s portroute %s\n", label, buf);
 	}
 }
 #else
 
-static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
@@ -68,19 +67,19 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
  * */
-static void dbg_hcc_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
 	u32	params = ehci_readl(ehci, &ehci->caps->hcc_params);
 
-	if (HCC_ISOC_CACHE (params)) {
-		ehci_dbg (ehci,
+	if (HCC_ISOC_CACHE(params)) {
+		ehci_dbg(ehci,
 			"%s hcc_params %04x caching frame %s%s%s\n",
 			label, params,
 			HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024",
 			HCC_CANPARK(params) ? " park" : "",
 			HCC_64BIT_ADDR(params) ? " 64 bit addr" : "");
 	} else {
-		ehci_dbg (ehci,
+		ehci_dbg(ehci,
 			"%s hcc_params %04x thresh %d uframes %s%s%s%s%s%s%s\n",
 			label,
 			params,
@@ -97,14 +96,14 @@ static void dbg_hcc_params (struct ehci_hcd *ehci, char *label)
 }
 #else
 
-static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
 
 #endif
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
 static void __maybe_unused
-dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
+dbg_qtd(const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
 {
 	ehci_dbg(ehci, "%s td %p n%08x %08x t%08x p0=%08x\n", label, qtd,
 		hc32_to_cpup(ehci, &qtd->hw_next),
@@ -120,22 +119,22 @@ dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
 }
 
 static void __maybe_unused
-dbg_qh (const char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
+dbg_qh(const char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
 	struct ehci_qh_hw *hw = qh->hw;
 
-	ehci_dbg (ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
+	ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
 		qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
 	dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
 }
 
 static void __maybe_unused
-dbg_itd (const char *label, struct ehci_hcd *ehci, struct ehci_itd *itd)
+dbg_itd(const char *label, struct ehci_hcd *ehci, struct ehci_itd *itd)
 {
-	ehci_dbg (ehci, "%s [%d] itd %p, next %08x, urb %p\n",
+	ehci_dbg(ehci, "%s [%d] itd %p, next %08x, urb %p\n",
 		label, itd->frame, itd, hc32_to_cpu(ehci, itd->hw_next),
 		itd->urb);
-	ehci_dbg (ehci,
+	ehci_dbg(ehci,
 		"  trans: %08x %08x %08x %08x %08x %08x %08x %08x\n",
 		hc32_to_cpu(ehci, itd->hw_transaction[0]),
 		hc32_to_cpu(ehci, itd->hw_transaction[1]),
@@ -145,7 +144,7 @@ dbg_itd (const char *label, struct ehci_hcd *ehci, struct ehci_itd *itd)
 		hc32_to_cpu(ehci, itd->hw_transaction[5]),
 		hc32_to_cpu(ehci, itd->hw_transaction[6]),
 		hc32_to_cpu(ehci, itd->hw_transaction[7]));
-	ehci_dbg (ehci,
+	ehci_dbg(ehci,
 		"  buf:   %08x %08x %08x %08x %08x %08x %08x\n",
 		hc32_to_cpu(ehci, itd->hw_bufp[0]),
 		hc32_to_cpu(ehci, itd->hw_bufp[1]),
@@ -154,19 +153,19 @@ dbg_itd (const char *label, struct ehci_hcd *ehci, struct ehci_itd *itd)
 		hc32_to_cpu(ehci, itd->hw_bufp[4]),
 		hc32_to_cpu(ehci, itd->hw_bufp[5]),
 		hc32_to_cpu(ehci, itd->hw_bufp[6]));
-	ehci_dbg (ehci, "  index: %d %d %d %d %d %d %d %d\n",
+	ehci_dbg(ehci, "  index: %d %d %d %d %d %d %d %d\n",
 		itd->index[0], itd->index[1], itd->index[2],
 		itd->index[3], itd->index[4], itd->index[5],
 		itd->index[6], itd->index[7]);
 }
 
 static void __maybe_unused
-dbg_sitd (const char *label, struct ehci_hcd *ehci, struct ehci_sitd *sitd)
+dbg_sitd(const char *label, struct ehci_hcd *ehci, struct ehci_sitd *sitd)
 {
-	ehci_dbg (ehci, "%s [%d] sitd %p, next %08x, urb %p\n",
+	ehci_dbg(ehci, "%s [%d] sitd %p, next %08x, urb %p\n",
 		label, sitd->frame, sitd, hc32_to_cpu(ehci, sitd->hw_next),
 		sitd->urb);
-	ehci_dbg (ehci,
+	ehci_dbg(ehci,
 		"  addr %08x sched %04x result %08x buf %08x %08x\n",
 		hc32_to_cpu(ehci, sitd->hw_fullspeed_ep),
 		hc32_to_cpu(ehci, sitd->hw_uframe),
@@ -176,9 +175,9 @@ dbg_sitd (const char *label, struct ehci_hcd *ehci, struct ehci_sitd *sitd)
 }
 
 static int __maybe_unused
-dbg_status_buf (char *buf, unsigned len, const char *label, u32 status)
+dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
 {
-	return scnprintf (buf, len,
+	return scnprintf(buf, len,
 		"%s%sstatus %04x%s%s%s%s%s%s%s%s%s%s%s",
 		label, label [0] ? " " : "", status,
 		(status & STS_PPCE_MASK) ? " PPCE" : "",
@@ -196,9 +195,9 @@ dbg_status_buf (char *buf, unsigned len, const char *label, u32 status)
 }
 
 static int __maybe_unused
-dbg_intr_buf (char *buf, unsigned len, const char *label, u32 enable)
+dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
 {
-	return scnprintf (buf, len,
+	return scnprintf(buf, len,
 		"%s%sintrenable %02x%s%s%s%s%s%s%s",
 		label, label [0] ? " " : "", enable,
 		(enable & STS_PPCE_MASK) ? " PPCE" : "",
@@ -215,9 +214,9 @@ static const char *const fls_strings [] =
     { "1024", "512", "256", "??" };
 
 static int
-dbg_command_buf (char *buf, unsigned len, const char *label, u32 command)
+dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
 {
-	return scnprintf (buf, len,
+	return scnprintf(buf, len,
 		"%s%scommand %07x %s%s%s%s%s%s=%d ithresh=%d%s%s%s%s "
 		"period=%s%s %s",
 		label, label [0] ? " " : "", command,
@@ -227,7 +226,7 @@ dbg_command_buf (char *buf, unsigned len, const char *label, u32 command)
 		(command & CMD_ASPE) ? " ASPE" : "",
 		(command & CMD_PSPE) ? " PSPE" : "",
 		(command & CMD_PARK) ? " park" : "(park)",
-		CMD_PARK_CNT (command),
+		CMD_PARK_CNT(command),
 		(command >> 16) & 0x3f,
 		(command & CMD_LRESET) ? " LReset" : "",
 		(command & CMD_IAAD) ? " IAAD" : "",
@@ -240,7 +239,7 @@ dbg_command_buf (char *buf, unsigned len, const char *label, u32 command)
 }
 
 static int
-dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
+dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 {
 	char	*sig;
 
@@ -252,7 +251,7 @@ dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
 	default: sig = "?"; break;
 	}
 
-	return scnprintf (buf, len,
+	return scnprintf(buf, len,
 		"%s%sport:%d status %06x %d %s%s%s%s%s%s "
 		"sig=%s%s%s%s%s%s%s%s%s%s%s",
 		label, label [0] ? " " : "", port, status,
@@ -282,23 +281,23 @@ dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
 
 #else
 static inline void __maybe_unused
-dbg_qh (char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
+dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
 {}
 
 static inline int __maybe_unused
-dbg_status_buf (char *buf, unsigned len, const char *label, u32 status)
+dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
 { return 0; }
 
 static inline int __maybe_unused
-dbg_command_buf (char *buf, unsigned len, const char *label, u32 command)
+dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
 { return 0; }
 
 static inline int __maybe_unused
-dbg_intr_buf (char *buf, unsigned len, const char *label, u32 enable)
+dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
 { return 0; }
 
 static inline int __maybe_unused
-dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
+dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 { return 0; }
 
 #endif	/* CONFIG_DYNAMIC_DEBUG */
@@ -326,8 +325,8 @@ dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status)
 
 #ifdef STUB_DEBUG_FILES
 
-static inline void create_debug_files (struct ehci_hcd *bus) { }
-static inline void remove_debug_files (struct ehci_hcd *bus) { }
+static inline void create_debug_files(struct ehci_hcd *bus) { }
+static inline void remove_debug_files(struct ehci_hcd *bus) { }
 
 #else
 
@@ -397,13 +396,13 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
 		return '*';
 	if (v & QTD_STS_HALT)
 		return '-';
-	if (!IS_SHORT_READ (v))
+	if (!IS_SHORT_READ(v))
 		return ' ';
 	/* tries to advance through hw_alt_next */
 	return '/';
 }
 
-static void qh_lines (
+static void qh_lines(
 	struct ehci_hcd *ehci,
 	struct ehci_qh *qh,
 	char **nextp,
@@ -435,7 +434,7 @@ static void qh_lines (
 	}
 	scratch = hc32_to_cpup(ehci, &hw->hw_info1);
 	hw_curr = (mark == '*') ? hc32_to_cpup(ehci, &hw->hw_current) : 0;
-	temp = scnprintf (next, size,
+	temp = scnprintf(next, size,
 			"qh/%p dev%d %cs ep%d %08x %08x (%08x%c %s nak%d)"
 			" [cur %08x next %08x buf[0] %08x]",
 			qh, scratch & 0x007f,
@@ -453,21 +452,21 @@ static void qh_lines (
 	next += temp;
 
 	/* hc may be modifying the list as we read it ... */
-	list_for_each (entry, &qh->qtd_list) {
-		td = list_entry (entry, struct ehci_qtd, qtd_list);
+	list_for_each(entry, &qh->qtd_list) {
+		td = list_entry(entry, struct ehci_qtd, qtd_list);
 		scratch = hc32_to_cpup(ehci, &td->hw_token);
 		mark = ' ';
 		if (hw_curr == td->qtd_dma)
 			mark = '*';
 		else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma))
 			mark = '+';
-		else if (QTD_LENGTH (scratch)) {
+		else if (QTD_LENGTH(scratch)) {
 			if (td->hw_alt_next == ehci->async->hw->hw_alt_next)
 				mark = '#';
 			else if (td->hw_alt_next != list_end)
 				mark = '/';
 		}
-		temp = snprintf (next, size,
+		temp = snprintf(next, size,
 				"\n\t%p%c%s len=%d %08x urb %p"
 				" [td %08x buf[0] %08x]",
 				td, mark, ({ char *tmp;
@@ -490,7 +489,7 @@ static void qh_lines (
 			goto done;
 	}
 
-	temp = snprintf (next, size, "\n");
+	temp = snprintf(next, size, "\n");
 	if (size < temp)
 		temp = size;
 	size -= temp;
@@ -511,7 +510,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 	struct ehci_qh		*qh;
 
 	hcd = bus_to_hcd(buf->bus);
-	ehci = hcd_to_ehci (hcd);
+	ehci = hcd_to_ehci(hcd);
 	next = buf->output_buf;
 	size = buf->alloc_size;
 
@@ -521,9 +520,9 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 	 * usually empty except for long-term bulk reads, or head.
 	 * one QH per line, and TDs we know about
 	 */
-	spin_lock_irqsave (&ehci->lock, flags);
+	spin_lock_irqsave(&ehci->lock, flags);
 	for (qh = ehci->async->qh_next.qh; size > 0 && qh; qh = qh->qh_next.qh)
-		qh_lines (ehci, qh, &next, &size);
+		qh_lines(ehci, qh, &next, &size);
 	if (!list_empty(&ehci->async_unlink) && size > 0) {
 		temp = scnprintf(next, size, "\nunlink =\n");
 		size -= temp;
@@ -535,7 +534,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 			qh_lines(ehci, qh, &next, &size);
 		}
 	}
-	spin_unlock_irqrestore (&ehci->lock, flags);
+	spin_unlock_irqrestore(&ehci->lock, flags);
 
 	return strlen(buf->output_buf);
 }
@@ -641,25 +640,25 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 	seen_count = 0;
 
 	hcd = bus_to_hcd(buf->bus);
-	ehci = hcd_to_ehci (hcd);
+	ehci = hcd_to_ehci(hcd);
 	next = buf->output_buf;
 	size = buf->alloc_size;
 
-	temp = scnprintf (next, size, "size = %d\n", ehci->periodic_size);
+	temp = scnprintf(next, size, "size = %d\n", ehci->periodic_size);
 	size -= temp;
 	next += temp;
 
 	/* dump a snapshot of the periodic schedule.
 	 * iso changes, interrupt usually doesn't.
 	 */
-	spin_lock_irqsave (&ehci->lock, flags);
+	spin_lock_irqsave(&ehci->lock, flags);
 	for (i = 0; i < ehci->periodic_size; i++) {
 		p = ehci->pshadow [i];
-		if (likely (!p.ptr))
+		if (likely(!p.ptr))
 			continue;
 		tag = Q_NEXT_TYPE(ehci, ehci->periodic [i]);
 
-		temp = scnprintf (next, size, "%4d: ", i);
+		temp = scnprintf(next, size, "%4d: ", i);
 		size -= temp;
 		next += temp;
 
@@ -669,7 +668,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 			switch (hc32_to_cpu(ehci, tag)) {
 			case Q_TYPE_QH:
 				hw = p.qh->hw;
-				temp = scnprintf (next, size, " qh%d-%04x/%p",
+				temp = scnprintf(next, size, " qh%d-%04x/%p",
 						p.qh->ps.period,
 						hc32_to_cpup(ehci,
 							&hw->hw_info2)
@@ -683,7 +682,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 					if (seen [temp].ptr != p.ptr)
 						continue;
 					if (p.qh->qh_next.ptr) {
-						temp = scnprintf (next, size,
+						temp = scnprintf(next, size,
 							" ...");
 						size -= temp;
 						next += temp;
@@ -699,7 +698,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 
 					/* count tds, get ep direction */
 					temp = 0;
-					list_for_each_entry (qtd,
+					list_for_each_entry(qtd,
 							&p.qh->qtd_list,
 							qtd_list) {
 						temp++;
@@ -711,7 +710,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 						}
 					}
 
-					temp = scnprintf (next, size,
+					temp = scnprintf(next, size,
 						" (%c%d ep%d%s "
 						"[%d/%d] q%d p%d)",
 						speed_char (scratch),
@@ -730,20 +729,20 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 				p = p.qh->qh_next;
 				break;
 			case Q_TYPE_FSTN:
-				temp = scnprintf (next, size,
+				temp = scnprintf(next, size,
 					" fstn-%8x/%p", p.fstn->hw_prev,
 					p.fstn);
 				tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
 				p = p.fstn->fstn_next;
 				break;
 			case Q_TYPE_ITD:
-				temp = scnprintf (next, size,
+				temp = scnprintf(next, size,
 					" itd/%p", p.itd);
 				tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
 				p = p.itd->itd_next;
 				break;
 			case Q_TYPE_SITD:
-				temp = scnprintf (next, size,
+				temp = scnprintf(next, size,
 					" sitd%d-%04x/%p",
 					p.sitd->stream->ps.period,
 					hc32_to_cpup(ehci, &p.sitd->hw_uframe)
@@ -757,12 +756,12 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 			next += temp;
 		} while (p.ptr);
 
-		temp = scnprintf (next, size, "\n");
+		temp = scnprintf(next, size, "\n");
 		size -= temp;
 		next += temp;
 	}
-	spin_unlock_irqrestore (&ehci->lock, flags);
-	kfree (seen);
+	spin_unlock_irqrestore(&ehci->lock, flags);
+	kfree(seen);
 
 	return buf->alloc_size - size;
 }
@@ -794,14 +793,14 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 	static char		label [] = "";
 
 	hcd = bus_to_hcd(buf->bus);
-	ehci = hcd_to_ehci (hcd);
+	ehci = hcd_to_ehci(hcd);
 	next = buf->output_buf;
 	size = buf->alloc_size;
 
-	spin_lock_irqsave (&ehci->lock, flags);
+	spin_lock_irqsave(&ehci->lock, flags);
 
 	if (!HCD_HW_ACCESSIBLE(hcd)) {
-		size = scnprintf (next, size,
+		size = scnprintf(next, size,
 			"bus %s, device %s\n"
 			"%s\n"
 			"SUSPENDED (no register access)\n",
@@ -813,7 +812,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 
 	/* Capability Registers */
 	i = HC_VERSION(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase));
-	temp = scnprintf (next, size,
+	temp = scnprintf(next, size,
 		"bus %s, device %s\n"
 		"%s\n"
 		"EHCI %x.%02x, rh state %s\n",
@@ -835,10 +834,10 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 		offset = HCC_EXT_CAPS(ehci_readl(ehci,
 				&ehci->caps->hcc_params));
 		while (offset && count--) {
-			pci_read_config_dword (pdev, offset, &cap);
+			pci_read_config_dword(pdev, offset, &cap);
 			switch (cap & 0xff) {
 			case 1:
-				temp = scnprintf (next, size,
+				temp = scnprintf(next, size,
 					"ownership %08x%s%s\n", cap,
 					(cap & (1 << 24)) ? " linux" : "",
 					(cap & (1 << 16)) ? " firmware" : "");
@@ -846,8 +845,8 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 				next += temp;
 
 				offset += 4;
-				pci_read_config_dword (pdev, offset, &cap2);
-				temp = scnprintf (next, size,
+				pci_read_config_dword(pdev, offset, &cap2);
+				temp = scnprintf(next, size,
 					"SMI sts/enable 0x%08x\n", cap2);
 				size -= temp;
 				next += temp;
@@ -865,48 +864,48 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 
 	// FIXME interpret both types of params
 	i = ehci_readl(ehci, &ehci->caps->hcs_params);
-	temp = scnprintf (next, size, "structural params 0x%08x\n", i);
+	temp = scnprintf(next, size, "structural params 0x%08x\n", i);
 	size -= temp;
 	next += temp;
 
 	i = ehci_readl(ehci, &ehci->caps->hcc_params);
-	temp = scnprintf (next, size, "capability params 0x%08x\n", i);
+	temp = scnprintf(next, size, "capability params 0x%08x\n", i);
 	size -= temp;
 	next += temp;
 
 	/* Operational Registers */
-	temp = dbg_status_buf (scratch, sizeof scratch, label,
+	temp = dbg_status_buf(scratch, sizeof(scratch), label,
 			ehci_readl(ehci, &ehci->regs->status));
-	temp = scnprintf (next, size, fmt, temp, scratch);
+	temp = scnprintf(next, size, fmt, temp, scratch);
 	size -= temp;
 	next += temp;
 
-	temp = dbg_command_buf (scratch, sizeof scratch, label,
+	temp = dbg_command_buf(scratch, sizeof(scratch), label,
 			ehci_readl(ehci, &ehci->regs->command));
-	temp = scnprintf (next, size, fmt, temp, scratch);
+	temp = scnprintf(next, size, fmt, temp, scratch);
 	size -= temp;
 	next += temp;
 
-	temp = dbg_intr_buf (scratch, sizeof scratch, label,
+	temp = dbg_intr_buf(scratch, sizeof(scratch), label,
 			ehci_readl(ehci, &ehci->regs->intr_enable));
-	temp = scnprintf (next, size, fmt, temp, scratch);
+	temp = scnprintf(next, size, fmt, temp, scratch);
 	size -= temp;
 	next += temp;
 
-	temp = scnprintf (next, size, "uframe %04x\n",
+	temp = scnprintf(next, size, "uframe %04x\n",
 			ehci_read_frame_index(ehci));
 	size -= temp;
 	next += temp;
 
-	for (i = 1; i <= HCS_N_PORTS (ehci->hcs_params); i++) {
-		temp = dbg_port_buf (scratch, sizeof scratch, label, i,
+	for (i = 1; i <= HCS_N_PORTS(ehci->hcs_params); i++) {
+		temp = dbg_port_buf(scratch, sizeof(scratch), label, i,
 				ehci_readl(ehci,
 					&ehci->regs->port_status[i - 1]));
-		temp = scnprintf (next, size, fmt, temp, scratch);
+		temp = scnprintf(next, size, fmt, temp, scratch);
 		size -= temp;
 		next += temp;
 		if (i == HCS_DEBUG_PORT(ehci->hcs_params) && ehci->debug) {
-			temp = scnprintf (next, size,
+			temp = scnprintf(next, size,
 					"    debug control %08x\n",
 					ehci_readl(ehci,
 						&ehci->debug->control));
@@ -924,21 +923,21 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 	}
 
 #ifdef EHCI_STATS
-	temp = scnprintf (next, size,
+	temp = scnprintf(next, size,
 		"irq normal %ld err %ld iaa %ld (lost %ld)\n",
 		ehci->stats.normal, ehci->stats.error, ehci->stats.iaa,
 		ehci->stats.lost_iaa);
 	size -= temp;
 	next += temp;
 
-	temp = scnprintf (next, size, "complete %ld unlink %ld\n",
+	temp = scnprintf(next, size, "complete %ld unlink %ld\n",
 		ehci->stats.complete, ehci->stats.unlink);
 	size -= temp;
 	next += temp;
 #endif
 
 done:
-	spin_unlock_irqrestore (&ehci->lock, flags);
+	spin_unlock_irqrestore(&ehci->lock, flags);
 
 	return buf->alloc_size - size;
 }
@@ -1054,7 +1053,7 @@ static int debug_registers_open(struct inode *inode, struct file *file)
 	return file->private_data ? 0 : -ENOMEM;
 }
 
-static inline void create_debug_files (struct ehci_hcd *ehci)
+static inline void create_debug_files(struct ehci_hcd *ehci)
 {
 	struct usb_bus *bus = &ehci_to_hcd(ehci)->self;
 
@@ -1084,7 +1083,7 @@ file_error:
 	debugfs_remove_recursive(ehci->debug_dir);
 }
 
-static inline void remove_debug_files (struct ehci_hcd *ehci)
+static inline void remove_debug_files(struct ehci_hcd *ehci)
 {
 	debugfs_remove_recursive(ehci->debug_dir);
 }
-- 
2.6.4

[PATCH 02/17] usb: host: ehci-dbg: remove space before open square bracket

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch. The only
change in this patch that isn't just removing spaces before opening
square brackets is at line 213 where the initialization of fls_strings[]
is placed in same line.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    Before this patch there are 20 warnings about spaces before open square
    bracket. After there are still 3. These lines concern to macros that
    will be modified to inline functions in sequential patches.
    
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index fcbbdfa..52bf3fe 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -42,7 +42,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 	/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
 	if (HCS_PORTROUTED(params)) {
 		int i;
-		char buf [46], tmp [7], byte;
+		char buf[46], tmp[7], byte;
 
 		buf[0] = 0;
 		for (i = 0; i < HCS_N_PORTS(params); i++) {
@@ -109,8 +109,8 @@ dbg_qtd(const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
 		hc32_to_cpup(ehci, &qtd->hw_next),
 		hc32_to_cpup(ehci, &qtd->hw_alt_next),
 		hc32_to_cpup(ehci, &qtd->hw_token),
-		hc32_to_cpup(ehci, &qtd->hw_buf [0]));
-	if (qtd->hw_buf [1])
+		hc32_to_cpup(ehci, &qtd->hw_buf[0]));
+	if (qtd->hw_buf[1])
 		ehci_dbg(ehci, "  p1=%08x p2=%08x p3=%08x p4=%08x\n",
 			hc32_to_cpup(ehci, &qtd->hw_buf[1]),
 			hc32_to_cpup(ehci, &qtd->hw_buf[2]),
@@ -179,7 +179,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
 {
 	return scnprintf(buf, len,
 		"%s%sstatus %04x%s%s%s%s%s%s%s%s%s%s%s",
-		label, label [0] ? " " : "", status,
+		label, label[0] ? " " : "", status,
 		(status & STS_PPCE_MASK) ? " PPCE" : "",
 		(status & STS_ASS) ? " Async" : "",
 		(status & STS_PSS) ? " Periodic" : "",
@@ -199,7 +199,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
 {
 	return scnprintf(buf, len,
 		"%s%sintrenable %02x%s%s%s%s%s%s%s",
-		label, label [0] ? " " : "", enable,
+		label, label[0] ? " " : "", enable,
 		(enable & STS_PPCE_MASK) ? " PPCE" : "",
 		(enable & STS_IAA) ? " IAA" : "",
 		(enable & STS_FATAL) ? " FATAL" : "",
@@ -210,8 +210,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
 		);
 }
 
-static const char *const fls_strings [] =
-    { "1024", "512", "256", "??" };
+static const char *const fls_strings[] = { "1024", "512", "256", "??" };
 
 static int
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
@@ -219,7 +218,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
 	return scnprintf(buf, len,
 		"%s%scommand %07x %s%s%s%s%s%s=%d ithresh=%d%s%s%s%s "
 		"period=%s%s %s",
-		label, label [0] ? " " : "", command,
+		label, label[0] ? " " : "", command,
 		(command & CMD_HIRD) ? " HIRD" : "",
 		(command & CMD_PPCEE) ? " PPCEE" : "",
 		(command & CMD_FSP) ? " FSP" : "",
@@ -232,7 +231,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
 		(command & CMD_IAAD) ? " IAAD" : "",
 		(command & CMD_ASE) ? " Async" : "",
 		(command & CMD_PSE) ? " Periodic" : "",
-		fls_strings [(command >> 2) & 0x3],
+		fls_strings[(command >> 2) & 0x3],
 		(command & CMD_RESET) ? " Reset" : "",
 		(command & CMD_RUN) ? "RUN" : "HALT"
 		);
@@ -254,7 +253,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 	return scnprintf(buf, len,
 		"%s%sport:%d status %06x %d %s%s%s%s%s%s "
 		"sig=%s%s%s%s%s%s%s%s%s%s%s",
-		label, label [0] ? " " : "", port, status,
+		label, label[0] ? " " : "", port, status,
 		status>>25,/*device address */
 		(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
 						" ACK" : "",
@@ -653,10 +652,10 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 	 */
 	spin_lock_irqsave(&ehci->lock, flags);
 	for (i = 0; i < ehci->periodic_size; i++) {
-		p = ehci->pshadow [i];
+		p = ehci->pshadow[i];
 		if (likely(!p.ptr))
 			continue;
-		tag = Q_NEXT_TYPE(ehci, ehci->periodic [i]);
+		tag = Q_NEXT_TYPE(ehci, ehci->periodic[i]);
 
 		temp = scnprintf(next, size, "%4d: ", i);
 		size -= temp;
@@ -679,7 +678,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 				next += temp;
 				/* don't repeat what follows this qh */
 				for (temp = 0; temp < seen_count; temp++) {
-					if (seen [temp].ptr != p.ptr)
+					if (seen[temp].ptr != p.ptr)
 						continue;
 					if (p.qh->qh_next.ptr) {
 						temp = scnprintf(next, size,
@@ -722,7 +721,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 						0x7ff & (scratch >> 16));
 
 					if (seen_count < DBG_SCHED_LIMIT)
-						seen [seen_count++].qh = p.qh;
+						seen[seen_count++].qh = p.qh;
 				} else
 					temp = 0;
 				tag = Q_NEXT_TYPE(ehci, hw->hw_next);
@@ -788,9 +787,9 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 	struct ehci_hcd		*ehci;
 	unsigned long		flags;
 	unsigned		temp, size, i;
-	char			*next, scratch [80];
-	static char		fmt [] = "%*s\n";
-	static char		label [] = "";
+	char			*next, scratch[80];
+	static char		fmt[] = "%*s\n";
+	static char		label[] = "";
 
 	hcd = bus_to_hcd(buf->bus);
 	ehci = hcd_to_ehci(hcd);
-- 
2.6.4

[PATCH 03/17] usb: host: ehci-dbg: use C89-style comments

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch.

Coding style demands usage of C89-style comments and a specific format
when it's multiline.

This also removes the Free Software Foundation address because FSF can
change it again.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 52bf3fe..df9f598 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -11,16 +11,14 @@
  * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
  * for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 /* this file is part of ehci-hcd.c */
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCSPARAMS register
+/*
+ * check the values in the HCSPARAMS register
  * (host controller _Structural_ parameters)
  * see EHCI spec, Table 2-4 for each value
  */
@@ -46,7 +44,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 
 		buf[0] = 0;
 		for (i = 0; i < HCS_N_PORTS(params); i++) {
-			// FIXME MIPS won't readb() ...
+			/* FIXME MIPS won't readb() ... */
 			byte = readb(&ehci->caps->portroute[(i >> 1)]);
 			sprintf(tmp, "%d ",
 				((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
@@ -63,10 +61,11 @@ static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
 
 #ifdef CONFIG_DYNAMIC_DEBUG
 
-/* check the values in the HCCPARAMS register
+/*
+ * check the values in the HCCPARAMS register
  * (host controller _Capability_ parameters)
  * see EHCI Spec, Table 2-5 for each value
- * */
+ */
 static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 {
 	u32	params = ehci_readl(ehci, &ehci->caps->hcc_params);
@@ -515,7 +514,8 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 
 	*next = 0;
 
-	/* dumps a snapshot of the async schedule.
+	/*
+	 * dumps a snapshot of the async schedule.
 	 * usually empty except for long-term bulk reads, or head.
 	 * one QH per line, and TDs we know about
 	 */
@@ -647,7 +647,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 	size -= temp;
 	next += temp;
 
-	/* dump a snapshot of the periodic schedule.
+	/*
+	 * dump a snapshot of the periodic schedule.
 	 * iso changes, interrupt usually doesn't.
 	 */
 	spin_lock_irqsave(&ehci->lock, flags);
@@ -861,7 +862,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 	}
 #endif
 
-	// FIXME interpret both types of params
+	/* FIXME interpret both types of params */
 	i = ehci_readl(ehci, &ehci->caps->hcs_params);
 	temp = scnprintf(next, size, "structural params 0x%08x\n", i);
 	size -= temp;
-- 
2.6.4

[PATCH 04/17] usb: host: ehci-dbg: move trailing statements to next line

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to switch case statements. There are few additional changes made to fix
other coding styles issues.

These additional changes are:

 - The compound statement "({...})" on line 474 is pulled out from
   snprintf parameters.

 - On line 723 the constant "0x03" is moved to right.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    Before this patch there are 14 warnings about trailing statements that
    should be on next line. After there are still 4. These lines concern to
    a macro that will be modified to inline function in sequential patches.
    
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 +++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index df9f598..c409e4f 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -243,10 +243,18 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 
 	/* signaling state */
 	switch (status & (3 << 10)) {
-	case 0 << 10: sig = "se0"; break;
-	case 1 << 10: sig = "k"; break;		/* low speed */
-	case 2 << 10: sig = "j"; break;
-	default: sig = "?"; break;
+	case 0 << 10:
+		sig = "se0";
+		break;
+	case 1 << 10: /* low speed */
+		sig = "k";
+		break;
+	case 2 << 10:
+		sig = "j";
+		break;
+	default:
+		sig = "?";
+		break;
 	}
 
 	return scnprintf(buf, len,
@@ -451,6 +459,8 @@ static void qh_lines(
 
 	/* hc may be modifying the list as we read it ... */
 	list_for_each(entry, &qh->qtd_list) {
+		char *type;
+
 		td = list_entry(entry, struct ehci_qtd, qtd_list);
 		scratch = hc32_to_cpup(ehci, &td->hw_token);
 		mark = ' ';
@@ -464,16 +474,24 @@ static void qh_lines(
 			else if (td->hw_alt_next != list_end)
 				mark = '/';
 		}
+		switch ((scratch >> 8) & 0x03) {
+		case 0:
+			type = "out";
+			break;
+		case 1:
+			type = "in";
+			break;
+		case 2:
+			type = "setup";
+			break;
+		default:
+			type = "?";
+			break;
+		}
 		temp = snprintf(next, size,
 				"\n\t%p%c%s len=%d %08x urb %p"
 				" [td %08x buf[0] %08x]",
-				td, mark, ({ char *tmp;
-				 switch ((scratch>>8)&0x03) {
-				 case 0: tmp = "out"; break;
-				 case 1: tmp = "in"; break;
-				 case 2: tmp = "setup"; break;
-				 default: tmp = "?"; break;
-				 } tmp;}),
+				td, mark, type,
 				(scratch >> 16) & 0x7fff,
 				scratch,
 				td->urb,
@@ -702,11 +720,15 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 							&p.qh->qtd_list,
 							qtd_list) {
 						temp++;
-						switch (0x03 & (hc32_to_cpu(
-							ehci,
-							qtd->hw_token) >> 8)) {
-						case 0: type = "out"; continue;
-						case 1: type = "in"; continue;
+						switch ((hc32_to_cpu(ehci,
+							qtd->hw_token) >> 8)
+								& 0x03) {
+						case 0:
+							type = "out";
+							continue;
+						case 1:
+							type = "in";
+							continue;
 						}
 					}
 
-- 
2.6.4

[PATCH 05/17] usb: host: ehci-dbg: fix up closing parenthesis

[Geyslan G. Bem]

This patch puts the closing parenthesis at the statement end removing
unnecessary "new line".

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c409e4f..3b423e1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -35,8 +35,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 		HCS_N_PCC(params),
 		HCS_PORTROUTED(params) ? "" : " ordered",
 		HCS_PPC(params) ? "" : " !ppc",
-		HCS_N_PORTS(params)
-		);
+		HCS_N_PORTS(params));
 	/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
 	if (HCS_PORTROUTED(params)) {
 		int i;
@@ -189,8 +188,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
 		(status & STS_FLR) ? " FLR" : "",
 		(status & STS_PCD) ? " PCD" : "",
 		(status & STS_ERR) ? " ERR" : "",
-		(status & STS_INT) ? " INT" : ""
-		);
+		(status & STS_INT) ? " INT" : "");
 }
 
 static int __maybe_unused
@@ -205,8 +203,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
 		(enable & STS_FLR) ? " FLR" : "",
 		(enable & STS_PCD) ? " PCD" : "",
 		(enable & STS_ERR) ? " ERR" : "",
-		(enable & STS_INT) ? " INT" : ""
-		);
+		(enable & STS_INT) ? " INT" : "");
 }
 
 static const char *const fls_strings[] = { "1024", "512", "256", "??" };
@@ -232,8 +229,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
 		(command & CMD_PSE) ? " Periodic" : "",
 		fls_strings[(command >> 2) & 0x3],
 		(command & CMD_RESET) ? " Reset" : "",
-		(command & CMD_RUN) ? "RUN" : "HALT"
-		);
+		(command & CMD_RUN) ? "RUN" : "HALT");
 }
 
 static int
-- 
2.6.4

[PATCH 06/17] usb: host: ehci-dbg: put spaces around operators

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to missing spaces around operators.

There is an additional change on line 49 that removes unnecessary
parenthesis around ternary operands.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 3b423e1..980ca55 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -46,7 +46,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 			/* FIXME MIPS won't readb() ... */
 			byte = readb(&ehci->caps->portroute[(i >> 1)]);
 			sprintf(tmp, "%d ",
-				((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
+				(i & 0x1) ? byte & 0xf : (byte >> 4) & 0xf);
 			strcat(buf, tmp);
 		}
 		ehci_dbg(ehci, "%s portroute %s\n", label, buf);
@@ -257,14 +257,14 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 		"%s%sport:%d status %06x %d %s%s%s%s%s%s "
 		"sig=%s%s%s%s%s%s%s%s%s%s%s",
 		label, label[0] ? " " : "", port, status,
-		status>>25,/*device address */
-		(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
+		status >> 25, /*device address */
+		(status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ACK ?
 						" ACK" : "",
-		(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_NYET ?
+		(status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_NYET ?
 						" NYET" : "",
-		(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_STALL ?
+		(status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_STALL ?
 						" STALL" : "",
-		(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ERR ?
+		(status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ERR ?
 						" ERR" : "",
 		(status & PORT_POWER) ? " POWER" : "",
 		(status & PORT_OWNER) ? " OWNER" : "",
@@ -846,7 +846,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
 	if (dev_is_pci(hcd->self.controller)) {
 		struct pci_dev	*pdev;
 		u32		offset, cap, cap2;
-		unsigned	count = 256/4;
+		unsigned	count = 256 / 4;
 
 		pdev = to_pci_dev(ehci_to_hcd(ehci)->self.controller);
 		offset = HCC_EXT_CAPS(ehci_readl(ehci,
@@ -1058,7 +1058,7 @@ static int debug_periodic_open(struct inode *inode, struct file *file)
 	if (!buf)
 		return -ENOMEM;
 
-	buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8)*PAGE_SIZE;
+	buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8) * PAGE_SIZE;
 	file->private_data = buf;
 	return 0;
 }
-- 
2.6.4

[PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison

[Geyslan G. Bem]

This patch fixes an unsigned comparison to less than 0.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    I'm not sure about that comparison because in qh_lines() temp receives
    the snprintf() return and thereafter occurs this comparison:
    
    if (size < temp)
       	   temp = size;
    
    Is it a test of string truncation right? That possibility is being
    treated. But if after some snprintf returns the temp is exactly size
    minus 1 (trailing null)? Could this scenario happen? If yes, I think
    size could be not counting correctly. Let me know more about it.

 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 980ca55..1645120 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 		next += temp;
 
 		list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
-			if (size <= 0)
+			if (size == 0)
 				break;
 			qh_lines(ehci, qh, &next, &size);
 		}
-- 
2.6.4

[PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to unnecessary space after a cast.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 1645120..edae79e 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 	ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
 		qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
-	dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
+	dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
 }
 
 static void __maybe_unused
@@ -491,7 +491,7 @@ static void qh_lines(
 				(scratch >> 16) & 0x7fff,
 				scratch,
 				td->urb,
-				(u32) td->qtd_dma,
+				(u32)td->qtd_dma,
 				hc32_to_cpup(ehci, &td->hw_buf[0]));
 		if (size < temp)
 			temp = size;
-- 
2.6.4

[PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Geyslan G. Bem]

Functions must have the opening brace at the beginning of the next line
and body conforming indentation.

This patch also reduces qh_lines() header definition to two lines.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index edae79e..a365d9d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
 }
 #else
 
-static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
+{
+}
 
 #endif
 
@@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
 }
 #else
 
-static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
+{
+}
 
 #endif
 
@@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 #else
 static inline void __maybe_unused
 dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
-{}
+{
+}
 
 static inline int __maybe_unused
 dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
-{ return 0; }
+{
+	return 0;
+}
 
 static inline int __maybe_unused
 dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
-{ return 0; }
+{
+	return 0;
+}
 
 static inline int __maybe_unused
 dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
-{ return 0; }
+{
+	return 0;
+}
 
 static inline int __maybe_unused
 dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
-{ return 0; }
+{
+	return 0;
+}
 
 #endif	/* CONFIG_DYNAMIC_DEBUG */
 
@@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 
 #ifdef STUB_DEBUG_FILES
 
-static inline void create_debug_files(struct ehci_hcd *bus) { }
-static inline void remove_debug_files(struct ehci_hcd *bus) { }
+static inline void create_debug_files(struct ehci_hcd *bus)
+{
+}
+
+static inline void remove_debug_files(struct ehci_hcd *bus)
+{
+}
 
 #else
 
@@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
 	return '/';
 }
 
-static void qh_lines(
-	struct ehci_hcd *ehci,
-	struct ehci_qh *qh,
-	char **nextp,
-	unsigned *sizep
-)
+static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
+			char **nextp, unsigned *sizep)
 {
 	u32			scratch;
 	u32			hw_curr;
-- 
2.6.4

[PATCH 10/17] usb: host: ehci-dbg: use a blank line after struct declarations

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to missing line after struct declarations.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index a365d9d..4ba577d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -367,6 +367,7 @@ static const struct file_operations debug_async_fops = {
 	.release	= debug_close,
 	.llseek		= default_llseek,
 };
+
 static const struct file_operations debug_bandwidth_fops = {
 	.owner		= THIS_MODULE,
 	.open		= debug_bandwidth_open,
@@ -374,6 +375,7 @@ static const struct file_operations debug_bandwidth_fops = {
 	.release	= debug_close,
 	.llseek		= default_llseek,
 };
+
 static const struct file_operations debug_periodic_fops = {
 	.owner		= THIS_MODULE,
 	.open		= debug_periodic_open,
@@ -381,6 +383,7 @@ static const struct file_operations debug_periodic_fops = {
 	.release	= debug_close,
 	.llseek		= default_llseek,
 };
+
 static const struct file_operations debug_registers_fops = {
 	.owner		= THIS_MODULE,
 	.open		= debug_registers_open,
-- 
2.6.4

[PATCH 11/17] usb: host: ehci-dbg: convert macro to inline function

[Geyslan G. Bem]

This patch converts macros into inline functions since the usage of
second is encouraged by Coding Style instead of the first.

Macros converted to functions:
 - dbg_status
 - dbg_cmd
 - dbg_port
 - speed_char

The size after changes remains the same.

Before:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

After:
text  data bss dec   hex  filename
36920 81   12  37013 9095 drivers/usb/host/ehci-hcd.o

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    The comment
    
    /* functions have the "wrong" filename when they're output... */
    
    was removed because after changes the file remained the same size and
    the symbols of the new inline functions were not created, so they were
    properly inlined.
    
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 54 ++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 4ba577d..db28992 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -317,23 +317,31 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
 
 #endif	/* CONFIG_DYNAMIC_DEBUG */
 
-/* functions have the "wrong" filename when they're output... */
-#define dbg_status(ehci, label, status) { \
-	char _buf [80]; \
-	dbg_status_buf (_buf, sizeof _buf, label, status); \
-	ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_status(struct ehci_hcd *ehci, const char *label, u32 status)
+{
+	char buf[80];
+
+	dbg_status_buf(buf, sizeof(buf), label, status);
+	ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_cmd(ehci, label, command) { \
-	char _buf [80]; \
-	dbg_command_buf (_buf, sizeof _buf, label, command); \
-	ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_cmd(struct ehci_hcd *ehci, const char *label, u32 command)
+{
+	char buf[80];
+
+	dbg_command_buf(buf, sizeof(buf), label, command);
+	ehci_dbg(ehci, "%s\n", buf);
 }
 
-#define dbg_port(ehci, label, port, status) { \
-	char _buf [80]; \
-	dbg_port_buf (_buf, sizeof _buf, label, port, status); \
-	ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_port(struct ehci_hcd *ehci, const char *label, int port, u32 status)
+{
+	char buf[80];
+
+	dbg_port_buf(buf, sizeof(buf), label, port, status);
+	ehci_dbg(ehci, "%s\n", buf);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -403,13 +411,19 @@ struct debug_buffer {
 	size_t alloc_size;
 };
 
-#define speed_char(info1) ({ char tmp; \
-		switch (info1 & (3 << 12)) { \
-		case QH_FULL_SPEED: tmp = 'f'; break; \
-		case QH_LOW_SPEED:  tmp = 'l'; break; \
-		case QH_HIGH_SPEED: tmp = 'h'; break; \
-		default: tmp = '?'; break; \
-		} tmp; })
+static inline char speed_char(u32 info1)
+{
+	switch (info1 & (3 << 12)) {
+	case QH_FULL_SPEED:
+		return 'f';
+	case QH_LOW_SPEED:
+		return 'l';
+	case QH_HIGH_SPEED:
+		return 'h';
+	default:
+		return '?';
+	}
+}
 
 static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
 {
-- 
2.6.4

[PATCH 12/17] usb: host: ehci-dbg: add blank line after declarations

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to missing line after variable declarations.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index db28992..a42b05b 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1085,6 +1085,7 @@ static int debug_bandwidth_open(struct inode *inode, struct file *file)
 static int debug_periodic_open(struct inode *inode, struct file *file)
 {
 	struct debug_buffer *buf;
+
 	buf = alloc_buffer(inode->i_private, fill_periodic_buffer);
 	if (!buf)
 		return -ENOMEM;
-- 
2.6.4

[PATCH 13/17] usb: host: ehci-dbg: remove blank line before close brace

[Geyslan G. Bem]

This patch fixes coding style issue reported by checkpatch concerning to
an unnecessary line before close brace.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index a42b05b..b9bbac7 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1052,7 +1052,6 @@ static ssize_t debug_output(struct file *file, char __user *user_buf,
 
 out:
 	return ret;
-
 }
 
 static int debug_close(struct inode *inode, struct file *file)
-- 
2.6.4

[PATCH 14/17] usb: host: ehci-dbg: replace sizeof operand

[Geyslan G. Bem]

This patch fixes a coding style issue reported by checkpatch concerning
to usage of sizeof operand as a variable instead the type.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b9bbac7..bf3ec19 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -996,7 +996,7 @@ static struct debug_buffer *alloc_buffer(struct usb_bus *bus,
 {
 	struct debug_buffer *buf;
 
-	buf = kzalloc(sizeof(struct debug_buffer), GFP_KERNEL);
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
 
 	if (buf) {
 		buf->bus = bus;
-- 
2.6.4

[PATCH 15/17] usb: host: ehci-dbg: enclose conditional blocks with braces

[Geyslan G. Bem]

This patch fixes coding style issues reported by checkpatch concerning
to conditional blocks without braces.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index bf3ec19..7f8ad18 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -491,11 +491,11 @@ static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
 		td = list_entry(entry, struct ehci_qtd, qtd_list);
 		scratch = hc32_to_cpup(ehci, &td->hw_token);
 		mark = ' ';
-		if (hw_curr == td->qtd_dma)
+		if (hw_curr == td->qtd_dma) {
 			mark = '*';
-		else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma))
+		} else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma)) {
 			mark = '+';
-		else if (QTD_LENGTH(scratch)) {
+		} else if (QTD_LENGTH(scratch)) {
 			if (td->hw_alt_next == ehci->async->hw->hw_alt_next)
 				mark = '#';
 			else if (td->hw_alt_next != list_end)
@@ -772,8 +772,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 
 					if (seen_count < DBG_SCHED_LIMIT)
 						seen[seen_count++].qh = p.qh;
-				} else
+				} else {
 					temp = 0;
+				}
 				tag = Q_NEXT_TYPE(ehci, hw->hw_next);
 				p = p.qh->qh_next;
 				break;
-- 
2.6.4

[PATCH 16/17] usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size

[Geyslan G. Bem]

This patch fixes a coding style issue reported by checkpatch related to
kmalloc_array usage.

On same line the sizeof operand was enclosed by parenthesis.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
 drivers/usb/host/ehci-dbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 7f8ad18..2268756 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -678,7 +678,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 	unsigned		i;
 	__hc32			tag;
 
-	seen = kmalloc(DBG_SCHED_LIMIT * sizeof *seen, GFP_ATOMIC);
+	seen = kmalloc_array(DBG_SCHED_LIMIT, sizeof(*seen), GFP_ATOMIC);
 	if (!seen)
 		return 0;
 	seen_count = 0;
-- 
2.6.4

[PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

[Geyslan G. Bem]

This patch fixes a coding style issue reported by checkpatch related to
many leading tabs, removing a 'do while' loop and making use of goto tag instead.

Others changes in this patch are:
 - Some multiline statements are reduced (718, 729, 780, 786, 790).
 - A constant is moved to right on line 770.

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---

Notes:
    Tested by compilation only.

 drivers/usb/host/ehci-dbg.c | 180 ++++++++++++++++++++++----------------------
 1 file changed, 88 insertions(+), 92 deletions(-)

diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 2268756..278333d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 	 */
 	spin_lock_irqsave(&ehci->lock, flags);
 	for (i = 0; i < ehci->periodic_size; i++) {
+		struct ehci_qh_hw *hw;
+
 		p = ehci->pshadow[i];
 		if (likely(!p.ptr))
 			continue;
@@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 		size -= temp;
 		next += temp;
 
-		do {
-			struct ehci_qh_hw *hw;
-
-			switch (hc32_to_cpu(ehci, tag)) {
-			case Q_TYPE_QH:
-				hw = p.qh->hw;
-				temp = scnprintf(next, size, " qh%d-%04x/%p",
-						p.qh->ps.period,
-						hc32_to_cpup(ehci,
-							&hw->hw_info2)
-							/* uframe masks */
-							& (QH_CMASK | QH_SMASK),
-						p.qh);
-				size -= temp;
-				next += temp;
-				/* don't repeat what follows this qh */
-				for (temp = 0; temp < seen_count; temp++) {
-					if (seen[temp].ptr != p.ptr)
+do_loop:
+		switch (hc32_to_cpu(ehci, tag)) {
+		case Q_TYPE_QH:
+			hw = p.qh->hw;
+			temp = scnprintf(next, size, " qh%d-%04x/%p",
+					p.qh->ps.period,
+					hc32_to_cpup(ehci, &hw->hw_info2)
+						/* uframe masks */
+						& (QH_CMASK | QH_SMASK),
+					p.qh);
+			size -= temp;
+			next += temp;
+			/* don't repeat what follows this qh */
+			for (temp = 0; temp < seen_count; temp++) {
+				if (seen[temp].ptr != p.ptr)
+					continue;
+				if (p.qh->qh_next.ptr) {
+					temp = scnprintf(next, size, " ...");
+					size -= temp;
+					next += temp;
+				}
+				break;
+			}
+			/* show more info the first time around */
+			if (temp == seen_count) {
+				u32	scratch = hc32_to_cpup(ehci,
+						&hw->hw_info1);
+				struct ehci_qtd	*qtd;
+				char		*type = "";
+
+				/* count tds, get ep direction */
+				temp = 0;
+				list_for_each_entry(qtd,
+						&p.qh->qtd_list,
+						qtd_list) {
+					temp++;
+					switch ((hc32_to_cpu(ehci,
+						qtd->hw_token) >> 8)
+							& 0x03) {
+					case 0:
+						type = "out";
+						continue;
+					case 1:
+						type = "in";
 						continue;
-					if (p.qh->qh_next.ptr) {
-						temp = scnprintf(next, size,
-							" ...");
-						size -= temp;
-						next += temp;
 					}
-					break;
 				}
-				/* show more info the first time around */
-				if (temp == seen_count) {
-					u32	scratch = hc32_to_cpup(ehci,
-							&hw->hw_info1);
-					struct ehci_qtd	*qtd;
-					char		*type = "";
-
-					/* count tds, get ep direction */
-					temp = 0;
-					list_for_each_entry(qtd,
-							&p.qh->qtd_list,
-							qtd_list) {
-						temp++;
-						switch ((hc32_to_cpu(ehci,
-							qtd->hw_token) >> 8)
-								& 0x03) {
-						case 0:
-							type = "out";
-							continue;
-						case 1:
-							type = "in";
-							continue;
-						}
-					}
 
-					temp = scnprintf(next, size,
-						" (%c%d ep%d%s "
-						"[%d/%d] q%d p%d)",
-						speed_char (scratch),
-						scratch & 0x007f,
-						(scratch >> 8) & 0x000f, type,
-						p.qh->ps.usecs,
-						p.qh->ps.c_usecs,
-						temp,
-						0x7ff & (scratch >> 16));
-
-					if (seen_count < DBG_SCHED_LIMIT)
-						seen[seen_count++].qh = p.qh;
-				} else {
-					temp = 0;
-				}
-				tag = Q_NEXT_TYPE(ehci, hw->hw_next);
-				p = p.qh->qh_next;
-				break;
-			case Q_TYPE_FSTN:
-				temp = scnprintf(next, size,
-					" fstn-%8x/%p", p.fstn->hw_prev,
-					p.fstn);
-				tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
-				p = p.fstn->fstn_next;
-				break;
-			case Q_TYPE_ITD:
-				temp = scnprintf(next, size,
-					" itd/%p", p.itd);
-				tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
-				p = p.itd->itd_next;
-				break;
-			case Q_TYPE_SITD:
 				temp = scnprintf(next, size,
-					" sitd%d-%04x/%p",
-					p.sitd->stream->ps.period,
-					hc32_to_cpup(ehci, &p.sitd->hw_uframe)
-						& 0x0000ffff,
-					p.sitd);
-				tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
-				p = p.sitd->sitd_next;
-				break;
+					" (%c%d ep%d%s "
+					"[%d/%d] q%d p%d)",
+					speed_char (scratch),
+					scratch & 0x007f,
+					(scratch >> 8) & 0x000f, type,
+					p.qh->ps.usecs,
+					p.qh->ps.c_usecs,
+					temp,
+					(scratch >> 16) & 0x7ff);
+
+				if (seen_count < DBG_SCHED_LIMIT)
+					seen[seen_count++].qh = p.qh;
+			} else {
+				temp = 0;
 			}
-			size -= temp;
-			next += temp;
-		} while (p.ptr);
+			tag = Q_NEXT_TYPE(ehci, hw->hw_next);
+			p = p.qh->qh_next;
+			break;
+		case Q_TYPE_FSTN:
+			temp = scnprintf(next, size, " fstn-%8x/%p",
+				p.fstn->hw_prev, p.fstn);
+			tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
+			p = p.fstn->fstn_next;
+			break;
+		case Q_TYPE_ITD:
+			temp = scnprintf(next, size, " itd/%p", p.itd);
+			tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
+			p = p.itd->itd_next;
+			break;
+		case Q_TYPE_SITD:
+			temp = scnprintf(next, size, " sitd%d-%04x/%p",
+				p.sitd->stream->ps.period,
+				hc32_to_cpup(ehci, &p.sitd->hw_uframe)
+					& 0x0000ffff,
+				p.sitd);
+			tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
+			p = p.sitd->sitd_next;
+			break;
+		}
+		size -= temp;
+		next += temp;
+		if (p.ptr)
+			goto do_loop;
 
 		temp = scnprintf(next, size, "\n");
 		size -= temp;
-- 
2.6.4

Re: [PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> This patch fixes an unsigned comparison to less than 0.

No, it doesn't.  It changes an unsigned comparison for less than or 
equal to 0, which is very different from less than 0.

> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
> 
> Notes:
>     I'm not sure about that comparison because in qh_lines() temp receives
>     the snprintf() return and thereafter occurs this comparison:
>     
>     if (size < temp)
>        	   temp = size;
>     
>     Is it a test of string truncation right? That possibility is being
>     treated. But if after some snprintf returns the temp is exactly size
>     minus 1 (trailing null)? Could this scenario happen? If yes, I think
>     size could be not counting correctly. Let me know more about it.

I think the two weird code sequences in qh_lines() were written before 
scnprintf existed.  They should be changed to use scnprintf instead of 
snprintf; then the "if (size < temp) temp = size;" things can be 
removed.

>  drivers/usb/host/ehci-dbg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 980ca55..1645120 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
>  		next += temp;
>  
>  		list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
> -			if (size <= 0)
> +			if (size == 0)
>  				break;
>  			qh_lines(ehci, qh, &next, &size);
>  		}

The new line does exactly the same thing as the old line.  There's no 
reason to make this change.

Alan Stern

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> This patch fixes coding style issues reported by checkpatch concerning
> to unnecessary space after a cast.

This is a case where checkpatch is wrong, IMO.  Casts should always be 
followed by a space.  I will not accept this patch.

This must be something recently added to checkpatch.  It never used to
complain about casts, whether they were followed by a space or not.

Alan Stern

> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  drivers/usb/host/ehci-dbg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 1645120..edae79e 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
>  
>  	ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
>  		qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
> -	dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
> +	dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
>  }
>  
>  static void __maybe_unused
> @@ -491,7 +491,7 @@ static void qh_lines(
>  				(scratch >> 16) & 0x7fff,
>  				scratch,
>  				td->urb,
> -				(u32) td->qtd_dma,
> +				(u32)td->qtd_dma,
>  				hc32_to_cpup(ehci, &td->hw_buf[0]));
>  		if (size < temp)
>  			temp = size;
> 

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> Functions must have the opening brace at the beginning of the next line
> and body conforming indentation.

This isn't necessary if the function is an empty static inline void 
routine.

Alan Stern

> This patch also reduces qh_lines() header definition to two lines.
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  drivers/usb/host/ehci-dbg.c | 44 +++++++++++++++++++++++++++++---------------
>  1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index edae79e..a365d9d 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
>  }
>  #else
>  
> -static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
> +static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
> +{
> +}
>  
>  #endif
>  
> @@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
>  }
>  #else
>  
> -static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
> +static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
> +{
> +}
>  
>  #endif
>  
> @@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
>  #else
>  static inline void __maybe_unused
>  dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
> -{}
> +{
> +}
>  
>  static inline int __maybe_unused
>  dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
> -{ return 0; }
> +{
> +	return 0;
> +}
>  
>  static inline int __maybe_unused
>  dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
> -{ return 0; }
> +{
> +	return 0;
> +}
>  
>  static inline int __maybe_unused
>  dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
> -{ return 0; }
> +{
> +	return 0;
> +}
>  
>  static inline int __maybe_unused
>  dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
> -{ return 0; }
> +{
> +	return 0;
> +}
>  
>  #endif	/* CONFIG_DYNAMIC_DEBUG */
>  
> @@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
>  
>  #ifdef STUB_DEBUG_FILES
>  
> -static inline void create_debug_files(struct ehci_hcd *bus) { }
> -static inline void remove_debug_files(struct ehci_hcd *bus) { }
> +static inline void create_debug_files(struct ehci_hcd *bus)
> +{
> +}
> +
> +static inline void remove_debug_files(struct ehci_hcd *bus)
> +{
> +}
>  
>  #else
>  
> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
>  	return '/';
>  }
>  
> -static void qh_lines(
> -	struct ehci_hcd *ehci,
> -	struct ehci_qh *qh,
> -	char **nextp,
> -	unsigned *sizep
> -)
> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> +			char **nextp, unsigned *sizep)
>  {
>  	u32			scratch;
>  	u32			hw_curr;
> 

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> This patch fixes a coding style issue reported by checkpatch related to
> many leading tabs, removing a 'do while' loop and making use of goto tag instead.

This is highly questionable.  It's a big amount of code churn, nearly 
impossible to verify visually, just to remove one level of indentation.  
It also introduces an unnecessary backwards "goto", which seems like a 
bad idea.

Alan Stern

> Others changes in this patch are:
>  - Some multiline statements are reduced (718, 729, 780, 786, 790).
>  - A constant is moved to right on line 770.
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
> 
> Notes:
>     Tested by compilation only.
> 
>  drivers/usb/host/ehci-dbg.c | 180 ++++++++++++++++++++++----------------------
>  1 file changed, 88 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 2268756..278333d 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  	 */
>  	spin_lock_irqsave(&ehci->lock, flags);
>  	for (i = 0; i < ehci->periodic_size; i++) {
> +		struct ehci_qh_hw *hw;
> +
>  		p = ehci->pshadow[i];
>  		if (likely(!p.ptr))
>  			continue;
> @@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  		size -= temp;
>  		next += temp;
>  
> -		do {
> -			struct ehci_qh_hw *hw;
> -
> -			switch (hc32_to_cpu(ehci, tag)) {
> -			case Q_TYPE_QH:
> -				hw = p.qh->hw;
> -				temp = scnprintf(next, size, " qh%d-%04x/%p",
> -						p.qh->ps.period,
> -						hc32_to_cpup(ehci,
> -							&hw->hw_info2)
> -							/* uframe masks */
> -							& (QH_CMASK | QH_SMASK),
> -						p.qh);
> -				size -= temp;
> -				next += temp;
> -				/* don't repeat what follows this qh */
> -				for (temp = 0; temp < seen_count; temp++) {
> -					if (seen[temp].ptr != p.ptr)
> +do_loop:
> +		switch (hc32_to_cpu(ehci, tag)) {
> +		case Q_TYPE_QH:
> +			hw = p.qh->hw;
> +			temp = scnprintf(next, size, " qh%d-%04x/%p",
> +					p.qh->ps.period,
> +					hc32_to_cpup(ehci, &hw->hw_info2)
> +						/* uframe masks */
> +						& (QH_CMASK | QH_SMASK),
> +					p.qh);
> +			size -= temp;
> +			next += temp;
> +			/* don't repeat what follows this qh */
> +			for (temp = 0; temp < seen_count; temp++) {
> +				if (seen[temp].ptr != p.ptr)
> +					continue;
> +				if (p.qh->qh_next.ptr) {
> +					temp = scnprintf(next, size, " ...");
> +					size -= temp;
> +					next += temp;
> +				}
> +				break;
> +			}
> +			/* show more info the first time around */
> +			if (temp == seen_count) {
> +				u32	scratch = hc32_to_cpup(ehci,
> +						&hw->hw_info1);
> +				struct ehci_qtd	*qtd;
> +				char		*type = "";
> +
> +				/* count tds, get ep direction */
> +				temp = 0;
> +				list_for_each_entry(qtd,
> +						&p.qh->qtd_list,
> +						qtd_list) {
> +					temp++;
> +					switch ((hc32_to_cpu(ehci,
> +						qtd->hw_token) >> 8)
> +							& 0x03) {
> +					case 0:
> +						type = "out";
> +						continue;
> +					case 1:
> +						type = "in";
>  						continue;
> -					if (p.qh->qh_next.ptr) {
> -						temp = scnprintf(next, size,
> -							" ...");
> -						size -= temp;
> -						next += temp;
>  					}
> -					break;
>  				}
> -				/* show more info the first time around */
> -				if (temp == seen_count) {
> -					u32	scratch = hc32_to_cpup(ehci,
> -							&hw->hw_info1);
> -					struct ehci_qtd	*qtd;
> -					char		*type = "";
> -
> -					/* count tds, get ep direction */
> -					temp = 0;
> -					list_for_each_entry(qtd,
> -							&p.qh->qtd_list,
> -							qtd_list) {
> -						temp++;
> -						switch ((hc32_to_cpu(ehci,
> -							qtd->hw_token) >> 8)
> -								& 0x03) {
> -						case 0:
> -							type = "out";
> -							continue;
> -						case 1:
> -							type = "in";
> -							continue;
> -						}
> -					}
>  
> -					temp = scnprintf(next, size,
> -						" (%c%d ep%d%s "
> -						"[%d/%d] q%d p%d)",
> -						speed_char (scratch),
> -						scratch & 0x007f,
> -						(scratch >> 8) & 0x000f, type,
> -						p.qh->ps.usecs,
> -						p.qh->ps.c_usecs,
> -						temp,
> -						0x7ff & (scratch >> 16));
> -
> -					if (seen_count < DBG_SCHED_LIMIT)
> -						seen[seen_count++].qh = p.qh;
> -				} else {
> -					temp = 0;
> -				}
> -				tag = Q_NEXT_TYPE(ehci, hw->hw_next);
> -				p = p.qh->qh_next;
> -				break;
> -			case Q_TYPE_FSTN:
> -				temp = scnprintf(next, size,
> -					" fstn-%8x/%p", p.fstn->hw_prev,
> -					p.fstn);
> -				tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
> -				p = p.fstn->fstn_next;
> -				break;
> -			case Q_TYPE_ITD:
> -				temp = scnprintf(next, size,
> -					" itd/%p", p.itd);
> -				tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
> -				p = p.itd->itd_next;
> -				break;
> -			case Q_TYPE_SITD:
>  				temp = scnprintf(next, size,
> -					" sitd%d-%04x/%p",
> -					p.sitd->stream->ps.period,
> -					hc32_to_cpup(ehci, &p.sitd->hw_uframe)
> -						& 0x0000ffff,
> -					p.sitd);
> -				tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
> -				p = p.sitd->sitd_next;
> -				break;
> +					" (%c%d ep%d%s "
> +					"[%d/%d] q%d p%d)",
> +					speed_char (scratch),
> +					scratch & 0x007f,
> +					(scratch >> 8) & 0x000f, type,
> +					p.qh->ps.usecs,
> +					p.qh->ps.c_usecs,
> +					temp,
> +					(scratch >> 16) & 0x7ff);
> +
> +				if (seen_count < DBG_SCHED_LIMIT)
> +					seen[seen_count++].qh = p.qh;
> +			} else {
> +				temp = 0;
>  			}
> -			size -= temp;
> -			next += temp;
> -		} while (p.ptr);
> +			tag = Q_NEXT_TYPE(ehci, hw->hw_next);
> +			p = p.qh->qh_next;
> +			break;
> +		case Q_TYPE_FSTN:
> +			temp = scnprintf(next, size, " fstn-%8x/%p",
> +				p.fstn->hw_prev, p.fstn);
> +			tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
> +			p = p.fstn->fstn_next;
> +			break;
> +		case Q_TYPE_ITD:
> +			temp = scnprintf(next, size, " itd/%p", p.itd);
> +			tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
> +			p = p.itd->itd_next;
> +			break;
> +		case Q_TYPE_SITD:
> +			temp = scnprintf(next, size, " sitd%d-%04x/%p",
> +				p.sitd->stream->ps.period,
> +				hc32_to_cpup(ehci, &p.sitd->hw_uframe)
> +					& 0x0000ffff,
> +				p.sitd);
> +			tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
> +			p = p.sitd->sitd_next;
> +			break;
> +		}
> +		size -= temp;
> +		next += temp;
> +		if (p.ptr)
> +			goto do_loop;
>  
>  		temp = scnprintf(next, size, "\n");
>  		size -= temp;
> 

Re: [PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison

[Geyslan G. Bem]

2016-01-04 17:50 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes an unsigned comparison to less than 0.
>
> No, it doesn't.  It changes an unsigned comparison for less than or
> equal to 0, which is very different from less than 0.
You're right. The statemant is incomplete.

>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>
>> Notes:
>>     I'm not sure about that comparison because in qh_lines() temp receives
>>     the snprintf() return and thereafter occurs this comparison:
>>
>>     if (size < temp)
>>                  temp = size;
>>
>>     Is it a test of string truncation right? That possibility is being
>>     treated. But if after some snprintf returns the temp is exactly size
>>     minus 1 (trailing null)? Could this scenario happen? If yes, I think
>>     size could be not counting correctly. Let me know more about it.
>
> I think the two weird code sequences in qh_lines() were written before
> scnprintf existed.  They should be changed to use scnprintf instead of
> snprintf; then the "if (size < temp) temp = size;" things can be
> removed.
I see. I can do another patch for that if you allow me.

>
>>  drivers/usb/host/ehci-dbg.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 980ca55..1645120 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
>>               next += temp;
>>
>>               list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
>> -                     if (size <= 0)
>> +                     if (size == 0)
>>                               break;
>>                       qh_lines(ehci, qh, &next, &size);
>>               }
>
> The new line does exactly the same thing as the old line.  There's no
> reason to make this change.
I think that the original and new logic will be the same because the
size variable has no sign. If in some previous subtraction the
subtracted value is greater than size value, this will spin (rotate),
probably, to a great positive value.

The compiled code will not change indeed. That change was only focused
on the improvement of the code reading. So if you allow me I could
change the commit message. If not let's forget it. :-)

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Geyslan G. Bem]

2016-01-04 17:58 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes coding style issues reported by checkpatch concerning
>> to unnecessary space after a cast.
>
> This is a case where checkpatch is wrong, IMO.  Casts should always be
> followed by a space.  I will not accept this patch.
Ok. I understand.

>
> This must be something recently added to checkpatch.  It never used to
> complain about casts, whether they were followed by a space or not.
I'm using the checkpatch --strict option.

>
> Alan Stern
>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>  drivers/usb/host/ehci-dbg.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 1645120..edae79e 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
>>
>>       ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
>>               qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
>> -     dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
>> +     dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
>>  }
>>
>>  static void __maybe_unused
>> @@ -491,7 +491,7 @@ static void qh_lines(
>>                               (scratch >> 16) & 0x7fff,
>>                               scratch,
>>                               td->urb,
>> -                             (u32) td->qtd_dma,
>> +                             (u32)td->qtd_dma,
>>                               hc32_to_cpup(ehci, &td->hw_buf[0]));
>>               if (size < temp)
>>                       temp = size;
>>
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Greg Kroah-Hartman]

On Mon, Jan 04, 2016 at 06:40:38PM -0300, Geyslan G. Bem wrote:
> 2016-01-04 17:58 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> >
> >> This patch fixes coding style issues reported by checkpatch concerning
> >> to unnecessary space after a cast.
> >
> > This is a case where checkpatch is wrong, IMO.  Casts should always be
> > followed by a space.  I will not accept this patch.
> Ok. I understand.
> 
> >
> > This must be something recently added to checkpatch.  It never used to
> > complain about casts, whether they were followed by a space or not.
> I'm using the checkpatch --strict option.

Please never use that on existing kernel code, except for
drivers/staging/ stuff, otherwise your patches will start to very
quickly be ignored.

thanks,

greg k-h

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Sergei Shtylyov]

Hello.

On 01/05/2016 12:49 AM, Greg Kroah-Hartman wrote:

>>>> This patch fixes coding style issues reported by checkpatch concerning
>>>> to unnecessary space after a cast.
>>>
>>> This is a case where checkpatch is wrong, IMO.  Casts should always be
>>> followed by a space.  I will not accept this patch.
>> Ok. I understand.
>>
>>>
>>> This must be something recently added to checkpatch.  It never used to
>>> complain about casts, whether they were followed by a space or not.
>> I'm using the checkpatch --strict option.
>
> Please never use that on existing kernel code, except for
> drivers/staging/ stuff, otherwise your patches will start to very
> quickly be ignored.

    Just wanted to remind everybody that this option is forced when checking 
the networking code...

> thanks,
>
> greg k-h

MBR, Sergei

Re: [PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> 2016-01-04 17:50 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> >
> >> This patch fixes an unsigned comparison to less than 0.
> >
> > No, it doesn't.  It changes an unsigned comparison for less than or
> > equal to 0, which is very different from less than 0.
> You're right. The statemant is incomplete.
> 
> >
> >> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> >> ---
> >>
> >> Notes:
> >>     I'm not sure about that comparison because in qh_lines() temp receives
> >>     the snprintf() return and thereafter occurs this comparison:
> >>
> >>     if (size < temp)
> >>                  temp = size;
> >>
> >>     Is it a test of string truncation right? That possibility is being
> >>     treated. But if after some snprintf returns the temp is exactly size
> >>     minus 1 (trailing null)? Could this scenario happen? If yes, I think
> >>     size could be not counting correctly. Let me know more about it.
> >
> > I think the two weird code sequences in qh_lines() were written before
> > scnprintf existed.  They should be changed to use scnprintf instead of
> > snprintf; then the "if (size < temp) temp = size;" things can be
> > removed.
> I see. I can do another patch for that if you allow me.

Sure, go ahead.

> >>  drivers/usb/host/ehci-dbg.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> >> index 980ca55..1645120 100644
> >> --- a/drivers/usb/host/ehci-dbg.c
> >> +++ b/drivers/usb/host/ehci-dbg.c
> >> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
> >>               next += temp;
> >>
> >>               list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
> >> -                     if (size <= 0)
> >> +                     if (size == 0)
> >>                               break;
> >>                       qh_lines(ehci, qh, &next, &size);
> >>               }
> >
> > The new line does exactly the same thing as the old line.  There's no
> > reason to make this change.
> I think that the original and new logic will be the same because the
> size variable has no sign. If in some previous subtraction the
> subtracted value is greater than size value, this will spin (rotate),
> probably, to a great positive value.
> 
> The compiled code will not change indeed. That change was only focused
> on the improvement of the code reading. So if you allow me I could
> change the commit message. If not let's forget it. :-)

IMO there's no improvement in reading the code.  So let's forget about 
this.

Alan Stern

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Geyslan G. Bem]

2016-01-04 18:52 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
> On 01/05/2016 12:49 AM, Greg Kroah-Hartman wrote:
>
>>>>> This patch fixes coding style issues reported by checkpatch concerning
>>>>> to unnecessary space after a cast.
>>>>
>>>>
>>>> This is a case where checkpatch is wrong, IMO.  Casts should always be
>>>> followed by a space.  I will not accept this patch.
>>>
>>> Ok. I understand.
>>>
>>>>
>>>> This must be something recently added to checkpatch.  It never used to
>>>> complain about casts, whether they were followed by a space or not.
>>>
>>> I'm using the checkpatch --strict option.
>>
>>
>> Please never use that on existing kernel code, except for
>> drivers/staging/ stuff, otherwise your patches will start to very
>> quickly be ignored.
Good to know. Tks.

>
>
>    Just wanted to remind everybody that this option is forced when checking
> the networking code...
Ditto.

>
>> thanks,
>>
>> greg k-h
>
>
> MBR, Sergei
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Geyslan G. Bem]

2016-01-04 18:00 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> Functions must have the opening brace at the beginning of the next line
>> and body conforming indentation.
>
> This isn't necessary if the function is an empty static inline void
> routine.
Ok. It's more related to style. Let's forget it.

>
> Alan Stern
>
>> This patch also reduces qh_lines() header definition to two lines.
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>  drivers/usb/host/ehci-dbg.c | 44 +++++++++++++++++++++++++++++---------------
>>  1 file changed, 29 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index edae79e..a365d9d 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
>>  }
>>  #else
>>
>> -static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
>> +static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
>> +{
>> +}
>>
>>  #endif
>>
>> @@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
>>  }
>>  #else
>>
>> -static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
>> +static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
>> +{
>> +}
>>
>>  #endif
>>
>> @@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
>>  #else
>>  static inline void __maybe_unused
>>  dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
>> -{}
>> +{
>> +}
>>
>>  static inline int __maybe_unused
>>  dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
>> -{ return 0; }
>> +{
>> +     return 0;
>> +}
>>
>>  static inline int __maybe_unused
>>  dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
>> -{ return 0; }
>> +{
>> +     return 0;
>> +}
>>
>>  static inline int __maybe_unused
>>  dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
>> -{ return 0; }
>> +{
>> +     return 0;
>> +}
>>
>>  static inline int __maybe_unused
>>  dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
>> -{ return 0; }
>> +{
>> +     return 0;
>> +}
>>
>>  #endif       /* CONFIG_DYNAMIC_DEBUG */
>>
>> @@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
>>
>>  #ifdef STUB_DEBUG_FILES
>>
>> -static inline void create_debug_files(struct ehci_hcd *bus) { }
>> -static inline void remove_debug_files(struct ehci_hcd *bus) { }
>> +static inline void create_debug_files(struct ehci_hcd *bus)
>> +{
>> +}
>> +
>> +static inline void remove_debug_files(struct ehci_hcd *bus)
>> +{
>> +}
>>
>>  #else
>>
>> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
>>       return '/';
>>  }
>>
>> -static void qh_lines(
>> -     struct ehci_hcd *ehci,
>> -     struct ehci_qh *qh,
>> -     char **nextp,
>> -     unsigned *sizep
>> -)
>> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>> +                     char **nextp, unsigned *sizep)
>>  {
>>       u32                     scratch;
>>       u32                     hw_curr;
>>
>
And about that style? Should be done?



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

[Geyslan G. Bem]

2016-01-04 18:01 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes a coding style issue reported by checkpatch related to
>> many leading tabs, removing a 'do while' loop and making use of goto tag instead.
>
> This is highly questionable.  It's a big amount of code churn, nearly
> impossible to verify visually, just to remove one level of indentation.
> It also introduces an unnecessary backwards "goto", which seems like a
> bad idea.
After hear you I agree. I saw that others drivers uses similar
structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
would be the case in this file of moving code to a new function? If
not, please disregard this patch.

>
> Alan Stern
>
>> Others changes in this patch are:
>>  - Some multiline statements are reduced (718, 729, 780, 786, 790).
>>  - A constant is moved to right on line 770.
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>
>> Notes:
>>     Tested by compilation only.
>>
>>  drivers/usb/host/ehci-dbg.c | 180 ++++++++++++++++++++++----------------------
>>  1 file changed, 88 insertions(+), 92 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 2268756..278333d 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>>        */
>>       spin_lock_irqsave(&ehci->lock, flags);
>>       for (i = 0; i < ehci->periodic_size; i++) {
>> +             struct ehci_qh_hw *hw;
>> +
>>               p = ehci->pshadow[i];
>>               if (likely(!p.ptr))
>>                       continue;
>> @@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>>               size -= temp;
>>               next += temp;
>>
>> -             do {
>> -                     struct ehci_qh_hw *hw;
>> -
>> -                     switch (hc32_to_cpu(ehci, tag)) {
>> -                     case Q_TYPE_QH:
>> -                             hw = p.qh->hw;
>> -                             temp = scnprintf(next, size, " qh%d-%04x/%p",
>> -                                             p.qh->ps.period,
>> -                                             hc32_to_cpup(ehci,
>> -                                                     &hw->hw_info2)
>> -                                                     /* uframe masks */
>> -                                                     & (QH_CMASK | QH_SMASK),
>> -                                             p.qh);
>> -                             size -= temp;
>> -                             next += temp;
>> -                             /* don't repeat what follows this qh */
>> -                             for (temp = 0; temp < seen_count; temp++) {
>> -                                     if (seen[temp].ptr != p.ptr)
>> +do_loop:
>> +             switch (hc32_to_cpu(ehci, tag)) {
>> +             case Q_TYPE_QH:
>> +                     hw = p.qh->hw;
>> +                     temp = scnprintf(next, size, " qh%d-%04x/%p",
>> +                                     p.qh->ps.period,
>> +                                     hc32_to_cpup(ehci, &hw->hw_info2)
>> +                                             /* uframe masks */
>> +                                             & (QH_CMASK | QH_SMASK),
>> +                                     p.qh);
>> +                     size -= temp;
>> +                     next += temp;
>> +                     /* don't repeat what follows this qh */
>> +                     for (temp = 0; temp < seen_count; temp++) {
>> +                             if (seen[temp].ptr != p.ptr)
>> +                                     continue;
>> +                             if (p.qh->qh_next.ptr) {
>> +                                     temp = scnprintf(next, size, " ...");
>> +                                     size -= temp;
>> +                                     next += temp;
>> +                             }
>> +                             break;
>> +                     }
>> +                     /* show more info the first time around */
>> +                     if (temp == seen_count) {
>> +                             u32     scratch = hc32_to_cpup(ehci,
>> +                                             &hw->hw_info1);
>> +                             struct ehci_qtd *qtd;
>> +                             char            *type = "";
>> +
>> +                             /* count tds, get ep direction */
>> +                             temp = 0;
>> +                             list_for_each_entry(qtd,
>> +                                             &p.qh->qtd_list,
>> +                                             qtd_list) {
>> +                                     temp++;
>> +                                     switch ((hc32_to_cpu(ehci,
>> +                                             qtd->hw_token) >> 8)
>> +                                                     & 0x03) {
>> +                                     case 0:
>> +                                             type = "out";
>> +                                             continue;
>> +                                     case 1:
>> +                                             type = "in";
>>                                               continue;
>> -                                     if (p.qh->qh_next.ptr) {
>> -                                             temp = scnprintf(next, size,
>> -                                                     " ...");
>> -                                             size -= temp;
>> -                                             next += temp;
>>                                       }
>> -                                     break;
>>                               }
>> -                             /* show more info the first time around */
>> -                             if (temp == seen_count) {
>> -                                     u32     scratch = hc32_to_cpup(ehci,
>> -                                                     &hw->hw_info1);
>> -                                     struct ehci_qtd *qtd;
>> -                                     char            *type = "";
>> -
>> -                                     /* count tds, get ep direction */
>> -                                     temp = 0;
>> -                                     list_for_each_entry(qtd,
>> -                                                     &p.qh->qtd_list,
>> -                                                     qtd_list) {
>> -                                             temp++;
>> -                                             switch ((hc32_to_cpu(ehci,
>> -                                                     qtd->hw_token) >> 8)
>> -                                                             & 0x03) {
>> -                                             case 0:
>> -                                                     type = "out";
>> -                                                     continue;
>> -                                             case 1:
>> -                                                     type = "in";
>> -                                                     continue;
>> -                                             }
>> -                                     }
>>
>> -                                     temp = scnprintf(next, size,
>> -                                             " (%c%d ep%d%s "
>> -                                             "[%d/%d] q%d p%d)",
>> -                                             speed_char (scratch),
>> -                                             scratch & 0x007f,
>> -                                             (scratch >> 8) & 0x000f, type,
>> -                                             p.qh->ps.usecs,
>> -                                             p.qh->ps.c_usecs,
>> -                                             temp,
>> -                                             0x7ff & (scratch >> 16));
>> -
>> -                                     if (seen_count < DBG_SCHED_LIMIT)
>> -                                             seen[seen_count++].qh = p.qh;
>> -                             } else {
>> -                                     temp = 0;
>> -                             }
>> -                             tag = Q_NEXT_TYPE(ehci, hw->hw_next);
>> -                             p = p.qh->qh_next;
>> -                             break;
>> -                     case Q_TYPE_FSTN:
>> -                             temp = scnprintf(next, size,
>> -                                     " fstn-%8x/%p", p.fstn->hw_prev,
>> -                                     p.fstn);
>> -                             tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
>> -                             p = p.fstn->fstn_next;
>> -                             break;
>> -                     case Q_TYPE_ITD:
>> -                             temp = scnprintf(next, size,
>> -                                     " itd/%p", p.itd);
>> -                             tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
>> -                             p = p.itd->itd_next;
>> -                             break;
>> -                     case Q_TYPE_SITD:
>>                               temp = scnprintf(next, size,
>> -                                     " sitd%d-%04x/%p",
>> -                                     p.sitd->stream->ps.period,
>> -                                     hc32_to_cpup(ehci, &p.sitd->hw_uframe)
>> -                                             & 0x0000ffff,
>> -                                     p.sitd);
>> -                             tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
>> -                             p = p.sitd->sitd_next;
>> -                             break;
>> +                                     " (%c%d ep%d%s "
>> +                                     "[%d/%d] q%d p%d)",
>> +                                     speed_char (scratch),
>> +                                     scratch & 0x007f,
>> +                                     (scratch >> 8) & 0x000f, type,
>> +                                     p.qh->ps.usecs,
>> +                                     p.qh->ps.c_usecs,
>> +                                     temp,
>> +                                     (scratch >> 16) & 0x7ff);
>> +
>> +                             if (seen_count < DBG_SCHED_LIMIT)
>> +                                     seen[seen_count++].qh = p.qh;
>> +                     } else {
>> +                             temp = 0;
>>                       }
>> -                     size -= temp;
>> -                     next += temp;
>> -             } while (p.ptr);
>> +                     tag = Q_NEXT_TYPE(ehci, hw->hw_next);
>> +                     p = p.qh->qh_next;
>> +                     break;
>> +             case Q_TYPE_FSTN:
>> +                     temp = scnprintf(next, size, " fstn-%8x/%p",
>> +                             p.fstn->hw_prev, p.fstn);
>> +                     tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
>> +                     p = p.fstn->fstn_next;
>> +                     break;
>> +             case Q_TYPE_ITD:
>> +                     temp = scnprintf(next, size, " itd/%p", p.itd);
>> +                     tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
>> +                     p = p.itd->itd_next;
>> +                     break;
>> +             case Q_TYPE_SITD:
>> +                     temp = scnprintf(next, size, " sitd%d-%04x/%p",
>> +                             p.sitd->stream->ps.period,
>> +                             hc32_to_cpup(ehci, &p.sitd->hw_uframe)
>> +                                     & 0x0000ffff,
>> +                             p.sitd);
>> +                     tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
>> +                     p = p.sitd->sitd_next;
>> +                     break;
>> +             }
>> +             size -= temp;
>> +             next += temp;
>> +             if (p.ptr)
>> +                     goto do_loop;
>>
>>               temp = scnprintf(next, size, "\n");
>>               size -= temp;
>>
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Joe Perches]

On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
> 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
> > > > > > This patch fixes coding style issues reported by checkpatch concerning
> > > > > > to unnecessary space after a cast.
> > > > > This is a case where checkpatch is wrong, IMO.  Casts should always be
> > > > > followed by a space.  I will not accept this patch.

Your choice, but most kernel code disagrees with you.

measuring only kernel casts to a pointer, (because there are
too many false positives otherwise) casts without a space
are preferred ~3:1 over casts followed by a space.

(without space)
$ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)\w+" * | \
  sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
36612

(with space)
$ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)[ \t]\w+" * | \
  sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
13233

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
> >>       return '/';
> >>  }
> >>
> >> -static void qh_lines(
> >> -     struct ehci_hcd *ehci,
> >> -     struct ehci_qh *qh,
> >> -     char **nextp,
> >> -     unsigned *sizep
> >> -)
> >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> >> +                     char **nextp, unsigned *sizep)
> >>  {
> >>       u32                     scratch;
> >>       u32                     hw_curr;
> >>
> >
> And about that style? Should be done?

You mean squeezing the function parameters into two lines?  That's 
okay.

However, the style in this file is to indent continuation lines by two
extra tab stops, not to line things up with an open paren on the first
line.

Alan Stern

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

[Alan Stern]

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> 2016-01-04 18:01 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> >
> >> This patch fixes a coding style issue reported by checkpatch related to
> >> many leading tabs, removing a 'do while' loop and making use of goto tag instead.
> >
> > This is highly questionable.  It's a big amount of code churn, nearly
> > impossible to verify visually, just to remove one level of indentation.
> > It also introduces an unnecessary backwards "goto", which seems like a
> > bad idea.
> After hear you I agree. I saw that others drivers uses similar
> structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
> would be the case in this file of moving code to a new function? If
> not, please disregard this patch.

Moving code into a new sub-function would be okay.

BTW, you don't need to post these patches to both linux-usb and LKML.  
linux-usb alone is good enough.  Nothing about the patches would be
especially interesting to a general Linux kernel programmer, so there's 
no point in bringing them to everybody's attention.

Alan Stern

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Geyslan G. Bem]

2016-01-04 23:40 GMT-03:00 Joe Perches <joe@perches.com>:
> On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
>> 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
>> > > > > > This patch fixes coding style issues reported by checkpatch concerning
>> > > > > > to unnecessary space after a cast.
>> > > > > This is a case where checkpatch is wrong, IMO.  Casts should always be
>> > > > > followed by a space.  I will not accept this patch.
>
> Your choice, but most kernel code disagrees with you.
>
> measuring only kernel casts to a pointer, (because there are
> too many false positives otherwise) casts without a space
> are preferred ~3:1 over casts followed by a space.
>
> (without space)
> $ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)\w+" * | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 36612
>
> (with space)
> $ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)[ \t]\w+" * | \
>   sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
> 13233

FWIW I really don't have a side here since it's not defined in Coding
Style. Please disregard this patch.

-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Joe Perches]

On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> 
> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct
> ehci_hcd *ehci, __hc32 token)
> > >>       return '/';
> > >>  }
> > >>
> > >> -static void qh_lines(
> > >> -     struct ehci_hcd *ehci,
> > >> -     struct ehci_qh *qh,
> > >> -     char **nextp,
> > >> -     unsigned *sizep
> > >> -)
> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > >> +                     char **nextp, unsigned *sizep)
> > >>  {
> > >>       u32                     scratch;
> > >>       u32                     hw_curr;
> > >>
> > >
> > And about that style? Should be done?
> 
> You mean squeezing the function parameters into two lines?  That's 
> okay.
> 
> However, the style in this file is to indent continuation lines by
> two
> extra tab stops, not to line things up with an open paren on the
> first
> line.
> 
> Alan Stern
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Joe Perches]

On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> 
> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
> > >>       return '/';
> > >>  }
> > >>
> > >> -static void qh_lines(
> > >> -     struct ehci_hcd *ehci,
> > >> -     struct ehci_qh *qh,
> > >> -     char **nextp,
> > >> -     unsigned *sizep
> > >> -)
> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > >> +                     char **nextp, unsigned *sizep)
> > >>  {
> > >>       u32                     scratch;
> > >>       u32                     hw_curr;
> > >>
> > >
> > And about that style? Should be done?
> 
> You mean squeezing the function parameters into two lines?  That's 
> okay.
> 
> However, the style in this file is to indent continuation lines by two
> extra tab stops, not to line things up with an open paren on the first
> line.

It's not consistent.
It's a bit of a mix of 1 and 2 tabs, and some others.

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Geyslan G. Bem]

2016-01-05 12:23 GMT-03:00 Joe Perches <joe@perches.com>:
> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
>> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>>
>> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
>> > >>       return '/';
>> > >>  }
>> > >>
>> > >> -static void qh_lines(
>> > >> -     struct ehci_hcd *ehci,
>> > >> -     struct ehci_qh *qh,
>> > >> -     char **nextp,
>> > >> -     unsigned *sizep
>> > >> -)
>> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>> > >> +                     char **nextp, unsigned *sizep)
>> > >>  {
>> > >>       u32                     scratch;
>> > >>       u32                     hw_curr;
>> > >>
>> > >
>> > And about that style? Should be done?
>>
>> You mean squeezing the function parameters into two lines?  That's
>> okay.
Yes. I'll change this patch to do only that squeezing.

>>
>> However, the style in this file is to indent continuation lines by two
>> extra tab stops, not to line things up with an open paren on the first
>> line.
I see. I used 3 tabs, reducing to 2.

>
> It's not consistent.
> It's a bit of a mix of 1 and 2 tabs, and some others.
I noticed it. Maybe to avoid the 80th column there are 1 tab indentations.

>
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Geyslan G. Bem]

2016-01-05 12:27 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> 2016-01-05 12:23 GMT-03:00 Joe Perches <joe@perches.com>:
>> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
>>> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>>>
>>> > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
>>> > >>       return '/';
>>> > >>  }
>>> > >>
>>> > >> -static void qh_lines(
>>> > >> -     struct ehci_hcd *ehci,
>>> > >> -     struct ehci_qh *qh,
>>> > >> -     char **nextp,
>>> > >> -     unsigned *sizep
>>> > >> -)
>>> > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>>> > >> +                     char **nextp, unsigned *sizep)
>>> > >>  {
>>> > >>       u32                     scratch;
>>> > >>       u32                     hw_curr;
>>> > >>
>>> > >
>>> > And about that style? Should be done?
>>>
>>> You mean squeezing the function parameters into two lines?  That's
>>> okay.
> Yes. I'll change this patch to do only that squeezing.
>
>>>
>>> However, the style in this file is to indent continuation lines by two
>>> extra tab stops, not to line things up with an open paren on the first
>>> line.
> I see. I used 3 tabs, reducing to 2.
>
>>
>> It's not consistent.
>> It's a bit of a mix of 1 and 2 tabs, and some others.
> I noticed it. Maybe to avoid the 80th column there are 1 tab indentations.

Others:

946
static struct debug_buffer *alloc_buffer(struct usb_bus *bus,
                ssize_t (*fill_func)(struct debug_buffer *))

has 4 tabs on the second line.

986
static ssize_t debug_output(struct file *file, char __user *user_buf,
                size_t len, loff_t *offset)

has 3 tabs and 4 spaces on the second line (aligned to open paren).

I think we should reduce them too to make the file consistent.

>
>>
>>
>
>
>
> --
> Regards,
>
> Geyslan G. Bem
> hackingbits.com



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function

[Geyslan G. Bem]

2016-01-05 12:15 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> 2016-01-04 18:01 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
>> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>> >
>> >> This patch fixes a coding style issue reported by checkpatch related to
>> >> many leading tabs, removing a 'do while' loop and making use of goto tag instead.
>> >
>> > This is highly questionable.  It's a big amount of code churn, nearly
>> > impossible to verify visually, just to remove one level of indentation.
>> > It also introduces an unnecessary backwards "goto", which seems like a
>> > bad idea.
>> After hear you I agree. I saw that others drivers uses similar
>> structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
>> would be the case in this file of moving code to a new function? If
>> not, please disregard this patch.
>
> Moving code into a new sub-function would be okay.
Ok.

>
> BTW, you don't need to post these patches to both linux-usb and LKML.
> linux-usb alone is good enough.  Nothing about the patches would be
> especially interesting to a general Linux kernel programmer, so there's
> no point in bringing them to everybody's attention.
You're right. I used git send-email
--cc-cmd="scripts/get_maintainer.pl -i". Next patchset will not be
sent to LKML.
Tks.

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1742 bytes --]

On Tue, 5 Jan 2016, Joe Perches wrote:

> On Tue, 2016-01-05 at 10:12 -0500, Alan Stern wrote:
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> > 
> > > >> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
> > > >>       return '/';
> > > >>  }
> > > >>
> > > >> -static void qh_lines(
> > > >> -     struct ehci_hcd *ehci,
> > > >> -     struct ehci_qh *qh,
> > > >> -     char **nextp,
> > > >> -     unsigned *sizep
> > > >> -)
> > > >> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> > > >> +                     char **nextp, unsigned *sizep)
> > > >>  {
> > > >>       u32                     scratch;
> > > >>       u32                     hw_curr;
> > > >>
> > > >
> > > And about that style? Should be done?
> > 
> > You mean squeezing the function parameters into two lines?  That's 
> > okay.
> > 
> > However, the style in this file is to indent continuation lines by two
> > extra tab stops, not to line things up with an open paren on the first
> > line.
> 
> It's not consistent.
> It's a bit of a mix of 1 and 2 tabs, and some others.

I know.  That's because the files were written by various people at 
various times and nobody tried to enforce a rigid consistent style.

I'm not even consistent all the time in the things that I write.  There 
are places (see drivers/usb/core/config.c) where I indented 
continuation lines by 4 spaces instead of 2 tab stops.  And there are 
places where a continuation of a continuation gets indented even 
farther.

Trying to come up with hard-and-fast rules for this sort of thing is 
pretty hopeless.  Even "Maximize readability" doesn't work too well, 
because different people find different things most readable.

Alan Stern

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Joe Perches]

On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
> Trying to come up with hard-and-fast rules for this sort of thing is 
> pretty hopeless.  Even "Maximize readability" doesn't work too well, 
> because different people find different things most readable.

Readability is mostly habituation.

Align to open parenthesis is relatively easy to implement,
but it can get out of hand too with long naming.

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 614 bytes --]

On Tue, 5 Jan 2016, Joe Perches wrote:

> On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
> > Trying to come up with hard-and-fast rules for this sort of thing is 
> > pretty hopeless.  Even "Maximize readability" doesn't work too well, 
> > because different people find different things most readable.
> 
> Readability is mostly habituation.

Indeed.

> Align to open parenthesis is relatively easy to implement,
> but it can get out of hand too with long naming.

That's why I dislike it.  Plus the fact that it results in
continuations of different lines being indented by different amounts.

Alan Stern

Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1017 bytes --]

On Mon, 4 Jan 2016, Joe Perches wrote:

> On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
> > 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
> > > > > > > This patch fixes coding style issues reported by checkpatch concerning
> > > > > > > to unnecessary space after a cast.
> > > > > > This is a case where checkpatch is wrong, IMO.  Casts should always be
> > > > > > followed by a space.  I will not accept this patch.
> 
> Your choice, but most kernel code disagrees with you.
> 
> measuring only kernel casts to a pointer, (because there are
> too many false positives otherwise) casts without a space
> are preferred ~3:1 over casts followed by a space.

Then most kernel code is implicitly in violation of CodingStyle.
I'm referring to the section that says (admittedly, in a different 
context):

			... all right-thinking people know that
	(a) K&R are _right_ and (b) K&R are right.

K&R, both the first and second editions, is very consistent about 
always putting a space after a cast.

Alan Stern

Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions

[Geyslan G. Bem]

2016-01-05 13:29 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Tue, 5 Jan 2016, Joe Perches wrote:
>
>> On Tue, 2016-01-05 at 11:06 -0500, Alan Stern wrote:
>> > Trying to come up with hard-and-fast rules for this sort of thing is
>> > pretty hopeless.  Even "Maximize readability" doesn't work too well,
>> > because different people find different things most readable.
>>
>> Readability is mostly habituation.
>
> Indeed.

Ditto.

>
>> Align to open parenthesis is relatively easy to implement,
>> but it can get out of hand too with long naming.

It's easy and it's consistent (the place of sequential lines).

>
> That's why I dislike it.  Plus the fact that it results in
> continuations of different lines being indented by different amounts.

The different amounts is the price to pay for that indentation, but it
will mostly make the style consistent and easy as Joe stated. I'm not
claiming for that changing, just debating. :-D
But IMHO standardization could make things easier.

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com