[PATCH 1/9v2] usb: host: ehci.h: remove space before comma

[PATCH 1/9v2] usb: host: ehci.h: remove space before comma

[Geyslan G. Bem]

Get rid of spaces before comma.

Caught by checkpatch: "ERROR: space prohibited before that ','"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index ec61aed..6a36ef4 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -852,13 +852,13 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
 /*-------------------------------------------------------------------------*/
 
 #define ehci_dbg(ehci, fmt, args...) \
-	dev_dbg(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
+	dev_dbg(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
 #define ehci_err(ehci, fmt, args...) \
-	dev_err(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
+	dev_err(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
 #define ehci_info(ehci, fmt, args...) \
-	dev_info(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
+	dev_info(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
 #define ehci_warn(ehci, fmt, args...) \
-	dev_warn(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
+	dev_warn(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
 
 
 #ifndef CONFIG_DYNAMIC_DEBUG
-- 
2.6.3

[PATCH 2/9v2] usb: host: ehci.h: remove space before function open parenthesis

[Geyslan G. Bem]

Get rid of space between function name and open parenthesis.

Caught by checkpatch: "WARNING: space prohibited between function name
and open parenthesis '('"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 6a36ef4..46982df 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -268,13 +268,13 @@ struct ehci_hcd {			/* one per controller */
 };
 
 /* convert between an HCD pointer and the corresponding EHCI_HCD */
-static inline struct ehci_hcd *hcd_to_ehci (struct usb_hcd *hcd)
+static inline struct ehci_hcd *hcd_to_ehci(struct usb_hcd *hcd)
 {
 	return (struct ehci_hcd *) (hcd->hcd_priv);
 }
-static inline struct usb_hcd *ehci_to_hcd (struct ehci_hcd *ehci)
+static inline struct usb_hcd *ehci_to_hcd(struct ehci_hcd *ehci)
 {
-	return container_of ((void *) ehci, struct usb_hcd, hcd_priv);
+	return container_of((void *) ehci, struct usb_hcd, hcd_priv);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -327,9 +327,9 @@ struct ehci_qtd {
 } __attribute__ ((aligned (32)));
 
 /* mask NakCnt+T in qh->hw_alt_next */
-#define QTD_MASK(ehci)	cpu_to_hc32 (ehci, ~0x1f)
+#define QTD_MASK(ehci)	cpu_to_hc32(ehci, ~0x1f)
 
-#define IS_SHORT_READ(token) (QTD_LENGTH (token) != 0 && QTD_PID (token) == 1)
+#define IS_SHORT_READ(token) (QTD_LENGTH(token) != 0 && QTD_PID(token) == 1)
 
 /*-------------------------------------------------------------------------*/
 
@@ -806,7 +806,7 @@ static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int operational)
 #define ehci_big_endian_desc(e)		((e)->big_endian_desc)
 
 /* cpu to ehci */
-static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
+static inline __hc32 cpu_to_hc32(const struct ehci_hcd *ehci, const u32 x)
 {
 	return ehci_big_endian_desc(ehci)
 		? (__force __hc32)cpu_to_be32(x)
@@ -814,14 +814,14 @@ static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
 }
 
 /* ehci to cpu */
-static inline u32 hc32_to_cpu (const struct ehci_hcd *ehci, const __hc32 x)
+static inline u32 hc32_to_cpu(const struct ehci_hcd *ehci, const __hc32 x)
 {
 	return ehci_big_endian_desc(ehci)
 		? be32_to_cpu((__force __be32)x)
 		: le32_to_cpu((__force __le32)x);
 }
 
-static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
+static inline u32 hc32_to_cpup(const struct ehci_hcd *ehci, const __hc32 *x)
 {
 	return ehci_big_endian_desc(ehci)
 		? be32_to_cpup((__force __be32 *)x)
@@ -831,18 +831,18 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
 #else
 
 /* cpu to ehci */
-static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
+static inline __hc32 cpu_to_hc32(const struct ehci_hcd *ehci, const u32 x)
 {
 	return cpu_to_le32(x);
 }
 
 /* ehci to cpu */
-static inline u32 hc32_to_cpu (const struct ehci_hcd *ehci, const __hc32 x)
+static inline u32 hc32_to_cpu(const struct ehci_hcd *ehci, const __hc32 x)
 {
 	return le32_to_cpu(x);
 }
 
-static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
+static inline u32 hc32_to_cpup(const struct ehci_hcd *ehci, const __hc32 *x)
 {
 	return le32_to_cpup(x);
 }
-- 
2.6.3

[PATCH 3/9v2] usb: host: ehci.h: remove space before open square bracket

[Geyslan G. Bem]

Get rid of space before open square bracket.

Caught by checkpatch: "ERROR: space prohibited before open square
bracket '['"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 46982df..cfeebd8 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -185,7 +185,7 @@ struct ehci_hcd {			/* one per controller */
 	struct ehci_sitd	*last_sitd_to_free;
 
 	/* per root hub port */
-	unsigned long		reset_done [EHCI_MAX_ROOT_PORTS];
+	unsigned long		reset_done[EHCI_MAX_ROOT_PORTS];
 
 	/* bit vectors (one bit per port) */
 	unsigned long		bus_suspended;		/* which ports were
@@ -316,8 +316,8 @@ struct ehci_qtd {
 #define HALT_BIT(ehci)		cpu_to_hc32(ehci, QTD_STS_HALT)
 #define STATUS_BIT(ehci)	cpu_to_hc32(ehci, QTD_STS_STS)
 
-	__hc32			hw_buf [5];        /* see EHCI 3.5.4 */
-	__hc32			hw_buf_hi [5];        /* Appendix B */
+	__hc32			hw_buf[5];        /* see EHCI 3.5.4 */
+	__hc32			hw_buf_hi[5];        /* Appendix B */
 
 	/* the rest is HCD-private */
 	dma_addr_t		qtd_dma;		/* qtd address */
@@ -405,8 +405,8 @@ struct ehci_qh_hw {
 	__hc32			hw_qtd_next;
 	__hc32			hw_alt_next;
 	__hc32			hw_token;
-	__hc32			hw_buf [5];
-	__hc32			hw_buf_hi [5];
+	__hc32			hw_buf[5];
+	__hc32			hw_buf_hi[5];
 } __attribute__ ((aligned(32)));
 
 struct ehci_qh {
@@ -510,7 +510,7 @@ struct ehci_iso_stream {
 struct ehci_itd {
 	/* first part defined by EHCI spec */
 	__hc32			hw_next;           /* see EHCI 3.3.1 */
-	__hc32			hw_transaction [8]; /* see EHCI 3.3.2 */
+	__hc32			hw_transaction[8]; /* see EHCI 3.3.2 */
 #define EHCI_ISOC_ACTIVE        (1<<31)        /* activate transfer this slot */
 #define EHCI_ISOC_BUF_ERR       (1<<30)        /* Data buffer error */
 #define EHCI_ISOC_BABBLE        (1<<29)        /* babble detected */
@@ -520,8 +520,8 @@ struct ehci_itd {
 
 #define ITD_ACTIVE(ehci)	cpu_to_hc32(ehci, EHCI_ISOC_ACTIVE)
 
-	__hc32			hw_bufp [7];	/* see EHCI 3.3.3 */
-	__hc32			hw_bufp_hi [7];	/* Appendix B */
+	__hc32			hw_bufp[7];	/* see EHCI 3.3.3 */
+	__hc32			hw_bufp_hi[7];	/* Appendix B */
 
 	/* the rest is HCD-private */
 	dma_addr_t		itd_dma;	/* for this itd */
@@ -565,9 +565,9 @@ struct ehci_sitd {
 
 #define SITD_ACTIVE(ehci)	cpu_to_hc32(ehci, SITD_STS_ACTIVE)
 
-	__hc32			hw_buf [2];		/* EHCI table 3-12 */
+	__hc32			hw_buf[2];		/* EHCI table 3-12 */
 	__hc32			hw_backpointer;		/* EHCI table 3-13 */
-	__hc32			hw_buf_hi [2];		/* Appendix B */
+	__hc32			hw_buf_hi[2];		/* Appendix B */
 
 	/* the rest is HCD-private */
 	dma_addr_t		sitd_dma;
-- 
2.6.3

[PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

Don't use the 'do {} while (0)' wrapper in a single statement macro.

Caught by checkpatch: "WARNING: Single statement macros should not
use a do {} while (0) loop"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index cfeebd8..945000a 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -244,9 +244,9 @@ struct ehci_hcd {			/* one per controller */
 	/* irq statistics */
 #ifdef EHCI_STATS
 	struct ehci_stats	stats;
-#	define COUNT(x) do { (x)++; } while (0)
+#	define COUNT(x) ((x)++)
 #else
-#	define COUNT(x) do {} while (0)
+#	define COUNT(x) ((void) 0)
 #endif
 
 	/* debug files */
-- 
2.6.3

[PATCH 5/9v2] usb: host: ehci.h: remove direct use of __attribute__ keyword

[Geyslan G. Bem]

Prefer to use __aligned(size) macro instead of
__attribute__((aligned(size))).

Caught by checkpatch: "WARNING"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 945000a..7b2b213 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -324,7 +324,7 @@ struct ehci_qtd {
 	struct list_head	qtd_list;		/* sw qtd list */
 	struct urb		*urb;			/* qtd's urb */
 	size_t			length;			/* length of buffer */
-} __attribute__ ((aligned (32)));
+} __aligned(32);
 
 /* mask NakCnt+T in qh->hw_alt_next */
 #define QTD_MASK(ehci)	cpu_to_hc32(ehci, ~0x1f)
@@ -407,7 +407,7 @@ struct ehci_qh_hw {
 	__hc32			hw_token;
 	__hc32			hw_buf[5];
 	__hc32			hw_buf_hi[5];
-} __attribute__ ((aligned(32)));
+} __aligned(32);
 
 struct ehci_qh {
 	struct ehci_qh_hw	*hw;		/* Must come first */
@@ -535,7 +535,7 @@ struct ehci_itd {
 	unsigned		frame;		/* where scheduled */
 	unsigned		pg;
 	unsigned		index[8];	/* in urb->iso_frame_desc */
-} __attribute__ ((aligned (32)));
+} __aligned(32);
 
 /*-------------------------------------------------------------------------*/
 
@@ -578,7 +578,7 @@ struct ehci_sitd {
 	struct list_head	sitd_list;	/* list of stream's sitds */
 	unsigned		frame;
 	unsigned		index;
-} __attribute__ ((aligned (32)));
+} __aligned(32);
 
 /*-------------------------------------------------------------------------*/
 
@@ -598,7 +598,7 @@ struct ehci_fstn {
 	/* the rest is HCD-private */
 	dma_addr_t		fstn_dma;
 	union ehci_shadow	fstn_next;	/* ptr to periodic q entry */
-} __attribute__ ((aligned (32)));
+} __aligned(32);
 
 /*-------------------------------------------------------------------------*/
 
-- 
2.6.3

[PATCH 6/9v2] usb: host: ehci.h: use space after comma

[Geyslan G. Bem]

Put space after comma.

This patch also changes QH_NEXT macro for better reading.

Caught by checkpatch: "ERROR: space required after that ','"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 7b2b213..ea56147 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -334,7 +334,7 @@ struct ehci_qtd {
 /*-------------------------------------------------------------------------*/
 
 /* type tag from {qh,itd,sitd,fstn}->hw_next */
-#define Q_NEXT_TYPE(ehci,dma)	((dma) & cpu_to_hc32(ehci, 3 << 1))
+#define Q_NEXT_TYPE(ehci, dma)	((dma) & cpu_to_hc32(ehci, 3 << 1))
 
 /*
  * Now the following defines are not converted using the
@@ -350,7 +350,8 @@ struct ehci_qtd {
 #define Q_TYPE_FSTN	(3 << 1)
 
 /* next async queue entry, or pointer to interrupt/periodic QH */
-#define QH_NEXT(ehci,dma)	(cpu_to_hc32(ehci, (((u32)dma)&~0x01f)|Q_TYPE_QH))
+#define QH_NEXT(ehci, dma) \
+		(cpu_to_hc32(ehci, (((u32) dma) & ~0x01f) | Q_TYPE_QH))
 
 /* for periodic/async schedules and qtd lists, mark end of list */
 #define EHCI_LIST_END(ehci)	cpu_to_hc32(ehci, 1) /* "null pointer" to hw */
-- 
2.6.3

[PATCH 7/9v2] usb: host: ehci.h: remove macros trailing semicolon

[Geyslan G. Bem]

Removes trailing semicolon from macros.

Caught by checkpatch:
"WARNING: macros should not use a trailing semicolon"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index ea56147..15de5bf 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -635,10 +635,10 @@ struct ehci_tt {
 /* Prepare the PORTSC wakeup flags during controller suspend/resume */
 
 #define ehci_prepare_ports_for_controller_suspend(ehci, do_wakeup)	\
-		ehci_adjust_port_wakeup_flags(ehci, true, do_wakeup);
+		ehci_adjust_port_wakeup_flags(ehci, true, do_wakeup)
 
 #define ehci_prepare_ports_for_controller_resume(ehci)			\
-		ehci_adjust_port_wakeup_flags(ehci, false, false);
+		ehci_adjust_port_wakeup_flags(ehci, false, false)
 
 /*-------------------------------------------------------------------------*/
 
-- 
2.6.3

[PATCH 8/9v2] usb: host: ehci.h: move pointer operator to name side

[Geyslan G. Bem]

The pointer operator must be sticked to name.

Caught by checkpatch:
ERROR: "foo * bar" should be "foo *bar"

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

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 15de5bf..81e609a 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -732,7 +732,7 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
 #endif
 
 static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
-		__u32 __iomem * regs)
+		__u32 __iomem *regs)
 {
 #ifdef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
 	return ehci_big_endian_mmio(ehci) ?
-- 
2.6.3

Re: [PATCH 1/9v2] usb: host: ehci.h: remove space before comma

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:30PM -0300, Geyslan G. Bem wrote:
> Get rid of spaces before comma.
> 
> Caught by checkpatch: "ERROR: space prohibited before that ','"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
It is a good idea to include here (after the ---) what changed from V1.
Other than that:
Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>

Not related to this patch, I did not receive patch 9/9v2.

>  drivers/usb/host/ehci.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index ec61aed..6a36ef4 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -852,13 +852,13 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
>  /*-------------------------------------------------------------------------*/
>  
>  #define ehci_dbg(ehci, fmt, args...) \
> -	dev_dbg(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +	dev_dbg(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
>  #define ehci_err(ehci, fmt, args...) \
> -	dev_err(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +	dev_err(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
>  #define ehci_info(ehci, fmt, args...) \
> -	dev_info(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +	dev_info(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
>  #define ehci_warn(ehci, fmt, args...) \
> -	dev_warn(ehci_to_hcd(ehci)->self.controller , fmt , ## args)
> +	dev_warn(ehci_to_hcd(ehci)->self.controller, fmt, ## args)
>  
>  
>  #ifndef CONFIG_DYNAMIC_DEBUG
> -- 
> 2.6.3
> 

Re: [PATCH 2/9v2] usb: host: ehci.h: remove space before function open parenthesis

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:31PM -0300, Geyslan G. Bem wrote:
> Get rid of space between function name and open parenthesis.
> 
> Caught by checkpatch: "WARNING: space prohibited between function name
> and open parenthesis '('"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
As with previous, missing the changelog here, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
>  drivers/usb/host/ehci.h | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 6a36ef4..46982df 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -268,13 +268,13 @@ struct ehci_hcd {			/* one per controller */
>  };
>  
>  /* convert between an HCD pointer and the corresponding EHCI_HCD */
> -static inline struct ehci_hcd *hcd_to_ehci (struct usb_hcd *hcd)
> +static inline struct ehci_hcd *hcd_to_ehci(struct usb_hcd *hcd)
>  {
>  	return (struct ehci_hcd *) (hcd->hcd_priv);
>  }
> -static inline struct usb_hcd *ehci_to_hcd (struct ehci_hcd *ehci)
> +static inline struct usb_hcd *ehci_to_hcd(struct ehci_hcd *ehci)
>  {
> -	return container_of ((void *) ehci, struct usb_hcd, hcd_priv);
> +	return container_of((void *) ehci, struct usb_hcd, hcd_priv);
>  }
>  
>  /*-------------------------------------------------------------------------*/
> @@ -327,9 +327,9 @@ struct ehci_qtd {
>  } __attribute__ ((aligned (32)));
>  
>  /* mask NakCnt+T in qh->hw_alt_next */
> -#define QTD_MASK(ehci)	cpu_to_hc32 (ehci, ~0x1f)
> +#define QTD_MASK(ehci)	cpu_to_hc32(ehci, ~0x1f)
>  
> -#define IS_SHORT_READ(token) (QTD_LENGTH (token) != 0 && QTD_PID (token) == 1)
> +#define IS_SHORT_READ(token) (QTD_LENGTH(token) != 0 && QTD_PID(token) == 1)
>  
>  /*-------------------------------------------------------------------------*/
>  
> @@ -806,7 +806,7 @@ static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int operational)
>  #define ehci_big_endian_desc(e)		((e)->big_endian_desc)
>  
>  /* cpu to ehci */
> -static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
> +static inline __hc32 cpu_to_hc32(const struct ehci_hcd *ehci, const u32 x)
>  {
>  	return ehci_big_endian_desc(ehci)
>  		? (__force __hc32)cpu_to_be32(x)
> @@ -814,14 +814,14 @@ static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
>  }
>  
>  /* ehci to cpu */
> -static inline u32 hc32_to_cpu (const struct ehci_hcd *ehci, const __hc32 x)
> +static inline u32 hc32_to_cpu(const struct ehci_hcd *ehci, const __hc32 x)
>  {
>  	return ehci_big_endian_desc(ehci)
>  		? be32_to_cpu((__force __be32)x)
>  		: le32_to_cpu((__force __le32)x);
>  }
>  
> -static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
> +static inline u32 hc32_to_cpup(const struct ehci_hcd *ehci, const __hc32 *x)
>  {
>  	return ehci_big_endian_desc(ehci)
>  		? be32_to_cpup((__force __be32 *)x)
> @@ -831,18 +831,18 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
>  #else
>  
>  /* cpu to ehci */
> -static inline __hc32 cpu_to_hc32 (const struct ehci_hcd *ehci, const u32 x)
> +static inline __hc32 cpu_to_hc32(const struct ehci_hcd *ehci, const u32 x)
>  {
>  	return cpu_to_le32(x);
>  }
>  
>  /* ehci to cpu */
> -static inline u32 hc32_to_cpu (const struct ehci_hcd *ehci, const __hc32 x)
> +static inline u32 hc32_to_cpu(const struct ehci_hcd *ehci, const __hc32 x)
>  {
>  	return le32_to_cpu(x);
>  }
>  
> -static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
> +static inline u32 hc32_to_cpup(const struct ehci_hcd *ehci, const __hc32 *x)
>  {
>  	return le32_to_cpup(x);
>  }
> -- 
> 2.6.3
> 

Re: [PATCH 3/9v2] usb: host: ehci.h: remove space before open square bracket

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:32PM -0300, Geyslan G. Bem wrote:
> Get rid of space before open square bracket.
> 
> Caught by checkpatch: "ERROR: space prohibited before open square
> bracket '['"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 46982df..cfeebd8 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -185,7 +185,7 @@ struct ehci_hcd {			/* one per controller */
>  	struct ehci_sitd	*last_sitd_to_free;
>  
>  	/* per root hub port */
> -	unsigned long		reset_done [EHCI_MAX_ROOT_PORTS];
> +	unsigned long		reset_done[EHCI_MAX_ROOT_PORTS];
>  
>  	/* bit vectors (one bit per port) */
>  	unsigned long		bus_suspended;		/* which ports were
> @@ -316,8 +316,8 @@ struct ehci_qtd {
>  #define HALT_BIT(ehci)		cpu_to_hc32(ehci, QTD_STS_HALT)
>  #define STATUS_BIT(ehci)	cpu_to_hc32(ehci, QTD_STS_STS)
>  
> -	__hc32			hw_buf [5];        /* see EHCI 3.5.4 */
> -	__hc32			hw_buf_hi [5];        /* Appendix B */
> +	__hc32			hw_buf[5];        /* see EHCI 3.5.4 */
> +	__hc32			hw_buf_hi[5];        /* Appendix B */
>  
>  	/* the rest is HCD-private */
>  	dma_addr_t		qtd_dma;		/* qtd address */
> @@ -405,8 +405,8 @@ struct ehci_qh_hw {
>  	__hc32			hw_qtd_next;
>  	__hc32			hw_alt_next;
>  	__hc32			hw_token;
> -	__hc32			hw_buf [5];
> -	__hc32			hw_buf_hi [5];
> +	__hc32			hw_buf[5];
> +	__hc32			hw_buf_hi[5];
>  } __attribute__ ((aligned(32)));
>  
>  struct ehci_qh {
> @@ -510,7 +510,7 @@ struct ehci_iso_stream {
>  struct ehci_itd {
>  	/* first part defined by EHCI spec */
>  	__hc32			hw_next;           /* see EHCI 3.3.1 */
> -	__hc32			hw_transaction [8]; /* see EHCI 3.3.2 */
> +	__hc32			hw_transaction[8]; /* see EHCI 3.3.2 */
>  #define EHCI_ISOC_ACTIVE        (1<<31)        /* activate transfer this slot */
>  #define EHCI_ISOC_BUF_ERR       (1<<30)        /* Data buffer error */
>  #define EHCI_ISOC_BABBLE        (1<<29)        /* babble detected */
> @@ -520,8 +520,8 @@ struct ehci_itd {
>  
>  #define ITD_ACTIVE(ehci)	cpu_to_hc32(ehci, EHCI_ISOC_ACTIVE)
>  
> -	__hc32			hw_bufp [7];	/* see EHCI 3.3.3 */
> -	__hc32			hw_bufp_hi [7];	/* Appendix B */
> +	__hc32			hw_bufp[7];	/* see EHCI 3.3.3 */
> +	__hc32			hw_bufp_hi[7];	/* Appendix B */
>  
>  	/* the rest is HCD-private */
>  	dma_addr_t		itd_dma;	/* for this itd */
> @@ -565,9 +565,9 @@ struct ehci_sitd {
>  
>  #define SITD_ACTIVE(ehci)	cpu_to_hc32(ehci, SITD_STS_ACTIVE)
>  
> -	__hc32			hw_buf [2];		/* EHCI table 3-12 */
> +	__hc32			hw_buf[2];		/* EHCI table 3-12 */
>  	__hc32			hw_backpointer;		/* EHCI table 3-13 */
> -	__hc32			hw_buf_hi [2];		/* Appendix B */
> +	__hc32			hw_buf_hi[2];		/* Appendix B */
>  
>  	/* the rest is HCD-private */
>  	dma_addr_t		sitd_dma;
> -- 
> 2.6.3
> 

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:33PM -0300, Geyslan G. Bem wrote:
> Don't use the 'do {} while (0)' wrapper in a single statement macro.
> 
> Caught by checkpatch: "WARNING: Single statement macros should not
> use a do {} while (0) loop"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index cfeebd8..945000a 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -244,9 +244,9 @@ struct ehci_hcd {			/* one per controller */
>  	/* irq statistics */
>  #ifdef EHCI_STATS
>  	struct ehci_stats	stats;
> -#	define COUNT(x) do { (x)++; } while (0)
> +#	define COUNT(x) ((x)++)
>  #else
> -#	define COUNT(x) do {} while (0)
> +#	define COUNT(x) ((void) 0)
>  #endif
>  
>  	/* debug files */
> -- 
> 2.6.3
> 

Re: [PATCH 5/9v2] usb: host: ehci.h: remove direct use of __attribute__ keyword

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:34PM -0300, Geyslan G. Bem wrote:
> Prefer to use __aligned(size) macro instead of
> __attribute__((aligned(size))).
> 
> Caught by checkpatch: "WARNING"
Not sure about the WARNING here, maybe just remove this line?

> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 945000a..7b2b213 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -324,7 +324,7 @@ struct ehci_qtd {
>  	struct list_head	qtd_list;		/* sw qtd list */
>  	struct urb		*urb;			/* qtd's urb */
>  	size_t			length;			/* length of buffer */
> -} __attribute__ ((aligned (32)));
> +} __aligned(32);
>  
>  /* mask NakCnt+T in qh->hw_alt_next */
>  #define QTD_MASK(ehci)	cpu_to_hc32(ehci, ~0x1f)
> @@ -407,7 +407,7 @@ struct ehci_qh_hw {
>  	__hc32			hw_token;
>  	__hc32			hw_buf[5];
>  	__hc32			hw_buf_hi[5];
> -} __attribute__ ((aligned(32)));
> +} __aligned(32);
>  
>  struct ehci_qh {
>  	struct ehci_qh_hw	*hw;		/* Must come first */
> @@ -535,7 +535,7 @@ struct ehci_itd {
>  	unsigned		frame;		/* where scheduled */
>  	unsigned		pg;
>  	unsigned		index[8];	/* in urb->iso_frame_desc */
> -} __attribute__ ((aligned (32)));
> +} __aligned(32);
>  
>  /*-------------------------------------------------------------------------*/
>  
> @@ -578,7 +578,7 @@ struct ehci_sitd {
>  	struct list_head	sitd_list;	/* list of stream's sitds */
>  	unsigned		frame;
>  	unsigned		index;
> -} __attribute__ ((aligned (32)));
> +} __aligned(32);
>  
>  /*-------------------------------------------------------------------------*/
>  
> @@ -598,7 +598,7 @@ struct ehci_fstn {
>  	/* the rest is HCD-private */
>  	dma_addr_t		fstn_dma;
>  	union ehci_shadow	fstn_next;	/* ptr to periodic q entry */
> -} __attribute__ ((aligned (32)));
> +} __aligned(32);
>  
>  /*-------------------------------------------------------------------------*/
>  
> -- 
> 2.6.3
> 

Re: [PATCH 6/9v2] usb: host: ehci.h: use space after comma

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:35PM -0300, Geyslan G. Bem wrote:
> Put space after comma.
> 
> This patch also changes QH_NEXT macro for better reading.
> 
> Caught by checkpatch: "ERROR: space required after that ','"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 7b2b213..ea56147 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -334,7 +334,7 @@ struct ehci_qtd {
>  /*-------------------------------------------------------------------------*/
>  
>  /* type tag from {qh,itd,sitd,fstn}->hw_next */
> -#define Q_NEXT_TYPE(ehci,dma)	((dma) & cpu_to_hc32(ehci, 3 << 1))
> +#define Q_NEXT_TYPE(ehci, dma)	((dma) & cpu_to_hc32(ehci, 3 << 1))
>  
>  /*
>   * Now the following defines are not converted using the
> @@ -350,7 +350,8 @@ struct ehci_qtd {
>  #define Q_TYPE_FSTN	(3 << 1)
>  
>  /* next async queue entry, or pointer to interrupt/periodic QH */
> -#define QH_NEXT(ehci,dma)	(cpu_to_hc32(ehci, (((u32)dma)&~0x01f)|Q_TYPE_QH))
> +#define QH_NEXT(ehci, dma) \
> +		(cpu_to_hc32(ehci, (((u32) dma) & ~0x01f) | Q_TYPE_QH))

For the maintainers: Is having two lines here better than having a line with 83 chars?

>  
>  /* for periodic/async schedules and qtd lists, mark end of list */
>  #define EHCI_LIST_END(ehci)	cpu_to_hc32(ehci, 1) /* "null pointer" to hw */
> -- 
> 2.6.3
> 

Re: [PATCH 7/9v2] usb: host: ehci.h: remove macros trailing semicolon

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:36PM -0300, Geyslan G. Bem wrote:
> Removes trailing semicolon from macros.
> 
> Caught by checkpatch:
> "WARNING: macros should not use a trailing semicolon"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index ea56147..15de5bf 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -635,10 +635,10 @@ struct ehci_tt {
>  /* Prepare the PORTSC wakeup flags during controller suspend/resume */
>  
>  #define ehci_prepare_ports_for_controller_suspend(ehci, do_wakeup)	\
> -		ehci_adjust_port_wakeup_flags(ehci, true, do_wakeup);
> +		ehci_adjust_port_wakeup_flags(ehci, true, do_wakeup)
>  
>  #define ehci_prepare_ports_for_controller_resume(ehci)			\
> -		ehci_adjust_port_wakeup_flags(ehci, false, false);
> +		ehci_adjust_port_wakeup_flags(ehci, false, false)
>  
>  /*-------------------------------------------------------------------------*/
>  
> -- 
> 2.6.3
> 

Re: [PATCH 8/9v2] usb: host: ehci.h: move pointer operator to name side

[Peter Senna Tschudin]

On Wed, Dec 09, 2015 at 07:32:37PM -0300, Geyslan G. Bem wrote:
> The pointer operator must be sticked to name.
> 
> Caught by checkpatch:
> ERROR: "foo * bar" should be "foo *bar"
> 
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/usb/host/ehci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 15de5bf..81e609a 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -732,7 +732,7 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
>  #endif
>  
>  static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
> -		__u32 __iomem * regs)
> +		__u32 __iomem *regs)
>  {
>  #ifdef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
>  	return ehci_big_endian_mmio(ehci) ?
> -- 
> 2.6.3
> 

Re: [PATCH 5/9v2] usb: host: ehci.h: remove direct use of __attribute__ keyword

[Geyslan G. Bem]

2015-12-10 7:01 GMT-03:00 Peter Senna Tschudin <peter.senna@gmail.com>:
> On Wed, Dec 09, 2015 at 07:32:34PM -0300, Geyslan G. Bem wrote:
>> Prefer to use __aligned(size) macro instead of
>> __attribute__((aligned(size))).
>>
>> Caught by checkpatch: "WARNING"
> Not sure about the WARNING here, maybe just remove this line?
Or just to be changed to "Caught by checkpatch."

>
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> As with previous, missing the changelog, but other than that the patch
> looks good to me.
>
> Acked-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> ---
>>  drivers/usb/host/ehci.h | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>> index 945000a..7b2b213 100644
>> --- a/drivers/usb/host/ehci.h
>> +++ b/drivers/usb/host/ehci.h
>> @@ -324,7 +324,7 @@ struct ehci_qtd {
>>       struct list_head        qtd_list;               /* sw qtd list */
>>       struct urb              *urb;                   /* qtd's urb */
>>       size_t                  length;                 /* length of buffer */
>> -} __attribute__ ((aligned (32)));
>> +} __aligned(32);
>>
>>  /* mask NakCnt+T in qh->hw_alt_next */
>>  #define QTD_MASK(ehci)       cpu_to_hc32(ehci, ~0x1f)
>> @@ -407,7 +407,7 @@ struct ehci_qh_hw {
>>       __hc32                  hw_token;
>>       __hc32                  hw_buf[5];
>>       __hc32                  hw_buf_hi[5];
>> -} __attribute__ ((aligned(32)));
>> +} __aligned(32);
>>
>>  struct ehci_qh {
>>       struct ehci_qh_hw       *hw;            /* Must come first */
>> @@ -535,7 +535,7 @@ struct ehci_itd {
>>       unsigned                frame;          /* where scheduled */
>>       unsigned                pg;
>>       unsigned                index[8];       /* in urb->iso_frame_desc */
>> -} __attribute__ ((aligned (32)));
>> +} __aligned(32);
>>
>>  /*-------------------------------------------------------------------------*/
>>
>> @@ -578,7 +578,7 @@ struct ehci_sitd {
>>       struct list_head        sitd_list;      /* list of stream's sitds */
>>       unsigned                frame;
>>       unsigned                index;
>> -} __attribute__ ((aligned (32)));
>> +} __aligned(32);
>>
>>  /*-------------------------------------------------------------------------*/
>>
>> @@ -598,7 +598,7 @@ struct ehci_fstn {
>>       /* the rest is HCD-private */
>>       dma_addr_t              fstn_dma;
>>       union ehci_shadow       fstn_next;      /* ptr to periodic q entry */
>> -} __attribute__ ((aligned (32)));
>> +} __aligned(32);
>>
>>  /*-------------------------------------------------------------------------*/
>>
>> --
>> 2.6.3
>>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Sergei Shtylyov]

Hello.

On 12/10/2015 1:32 AM, Geyslan G. Bem wrote:

> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>
> Caught by checkpatch: "WARNING: Single statement macros should not
> use a do {} while (0) loop"
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
>   drivers/usb/host/ehci.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index cfeebd8..945000a 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -244,9 +244,9 @@ struct ehci_hcd {			/* one per controller */
>   	/* irq statistics */
>   #ifdef EHCI_STATS
>   	struct ehci_stats	stats;
> -#	define COUNT(x) do { (x)++; } while (0)
> +#	define COUNT(x) ((x)++)
>   #else
> -#	define COUNT(x) do {} while (0)
> +#	define COUNT(x) ((void) 0)

    Why not just empty #define?

[...]

MBR, Sergei

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

2015-12-10 8:54 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
> On 12/10/2015 1:32 AM, Geyslan G. Bem wrote:
>
>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>
>> Caught by checkpatch: "WARNING: Single statement macros should not
>> use a do {} while (0) loop"
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>> ---
>>   drivers/usb/host/ehci.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>> index cfeebd8..945000a 100644
>> --- a/drivers/usb/host/ehci.h
>> +++ b/drivers/usb/host/ehci.h
>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>> controller */
>>         /* irq statistics */
>>   #ifdef EHCI_STATS
>>         struct ehci_stats       stats;
>> -#      define COUNT(x) do { (x)++; } while (0)
>> +#      define COUNT(x) ((x)++)
>>   #else
>> -#      define COUNT(x) do {} while (0)
>> +#      define COUNT(x) ((void) 0)
>
>
>    Why not just empty #define?

Indeed. I'll change it.
Tks Sergei.


>
> [...]
>
> MBR, Sergei
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

2015-12-10 9:20 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> 2015-12-10 8:54 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
>> Hello.
>>
>> On 12/10/2015 1:32 AM, Geyslan G. Bem wrote:
>>
>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>
>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>> use a do {} while (0) loop"
>>>
>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>> ---
>>>   drivers/usb/host/ehci.h | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>> index cfeebd8..945000a 100644
>>> --- a/drivers/usb/host/ehci.h
>>> +++ b/drivers/usb/host/ehci.h
>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>> controller */
>>>         /* irq statistics */
>>>   #ifdef EHCI_STATS
>>>         struct ehci_stats       stats;
>>> -#      define COUNT(x) do { (x)++; } while (0)
>>> +#      define COUNT(x) ((x)++)
>>>   #else
>>> -#      define COUNT(x) do {} while (0)
>>> +#      define COUNT(x) ((void) 0)
>>
>>
>>    Why not just empty #define?
>
> Indeed. I'll change it.
> Tks Sergei.

Since COUNT is not used to return the empty #define is ok. Another way
is to use #define COUNT(x) (0) to get a 0 when necessary to read
returns.

>
>
>>
>> [...]
>>
>> MBR, Sergei
>>
>
>
>
> --
> Regards,
>
> Geyslan G. Bem
> hackingbits.com



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Sergei Shtylyov]

On 12/10/2015 04:05 PM, Geyslan G. Bem wrote:

>>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>>
>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>> use a do {} while (0) loop"
>>>>
>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>> ---
>>>>    drivers/usb/host/ehci.h | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>> index cfeebd8..945000a 100644
>>>> --- a/drivers/usb/host/ehci.h
>>>> +++ b/drivers/usb/host/ehci.h
>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>> controller */
>>>>          /* irq statistics */
>>>>    #ifdef EHCI_STATS
>>>>          struct ehci_stats       stats;
>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>> +#      define COUNT(x) ((x)++)
>>>>    #else
>>>> -#      define COUNT(x) do {} while (0)
>>>> +#      define COUNT(x) ((void) 0)
>>>
>>>
>>>     Why not just empty #define?
>>
>> Indeed. I'll change it.
>> Tks Sergei.
>
> Since COUNT is not used to return the empty #define is ok. Another way
> is to use #define COUNT(x) (0) to get a 0 when necessary to read
> returns.

    Just 0, no parens please.

MBR, Sergei

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

2015-12-10 11:44 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> On 12/10/2015 04:05 PM, Geyslan G. Bem wrote:
>
>>>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>>>
>>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>>> use a do {} while (0) loop"
>>>>>
>>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>>> ---
>>>>>    drivers/usb/host/ehci.h | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>>> index cfeebd8..945000a 100644
>>>>> --- a/drivers/usb/host/ehci.h
>>>>> +++ b/drivers/usb/host/ehci.h
>>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>>> controller */
>>>>>          /* irq statistics */
>>>>>    #ifdef EHCI_STATS
>>>>>          struct ehci_stats       stats;
>>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>>> +#      define COUNT(x) ((x)++)
>>>>>    #else
>>>>> -#      define COUNT(x) do {} while (0)
>>>>> +#      define COUNT(x) ((void) 0)
>>>>
>>>>
>>>>
>>>>     Why not just empty #define?
>>>
>>>
>>> Indeed. I'll change it.
>>> Tks Sergei.
>>
>>
>> Since COUNT is not used to return the empty #define is ok. Another way
>> is to use #define COUNT(x) (0) to get a 0 when necessary to read
>> returns.
>
>
>    Just 0, no parens please.
Ok, no parens, since there's no evaluation.

Then my change is:

-#      define COUNT(x) do { (x)++; } while (0)
+#      define COUNT(x) (++(x))
 #else
-#      define COUNT(x) do {} while (0)
+#      define COUNT(x) 0

Pre-increment allowing to return the updated x.

>
> MBR, Sergei
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 6/9v2] usb: host: ehci.h: use space after comma

[Alan Stern]

On Thu, 10 Dec 2015, Peter Senna Tschudin wrote:

> >  /* next async queue entry, or pointer to interrupt/periodic QH */
> > -#define QH_NEXT(ehci,dma)	(cpu_to_hc32(ehci, (((u32)dma)&~0x01f)|Q_TYPE_QH))
> > +#define QH_NEXT(ehci, dma) \
> > +		(cpu_to_hc32(ehci, (((u32) dma) & ~0x01f) | Q_TYPE_QH))
> 
> For the maintainers: Is having two lines here better than having a line with 83 chars?

Two lines is better.  This patch is fine and so are the other 7 -- I 
never received 9/9v2 (the original 9/9 was okay).

Alan Stern

Re: [PATCH 6/9v2] usb: host: ehci.h: use space after comma

[Geyslan G. Bem]

2015-12-10 12:57 GMT-03:00 Alan Stern <stern@rowland.harvard.edu>:
> On Thu, 10 Dec 2015, Peter Senna Tschudin wrote:
>
>> >  /* next async queue entry, or pointer to interrupt/periodic QH */
>> > -#define QH_NEXT(ehci,dma)  (cpu_to_hc32(ehci, (((u32)dma)&~0x01f)|Q_TYPE_QH))
>> > +#define QH_NEXT(ehci, dma) \
>> > +           (cpu_to_hc32(ehci, (((u32) dma) & ~0x01f) | Q_TYPE_QH))
>>
>> For the maintainers: Is having two lines here better than having a line with 83 chars?
>
> Two lines is better.  This patch is fine and so are the other 7 -- I
> never received 9/9v2 (the original 9/9 was okay).
Alan,

The 9 is v1 only, since it mentions the tool (coccinelle) correctly.

Tks.

>
> Alan Stern
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Sergei Shtylyov]

On 12/10/2015 05:56 PM, Geyslan G. Bem wrote:

>>>>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>>>>
>>>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>>>> use a do {} while (0) loop"
>>>>>>
>>>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>>>> ---
>>>>>>     drivers/usb/host/ehci.h | 4 ++--
>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>>>> index cfeebd8..945000a 100644
>>>>>> --- a/drivers/usb/host/ehci.h
>>>>>> +++ b/drivers/usb/host/ehci.h
>>>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>>>> controller */
>>>>>>           /* irq statistics */
>>>>>>     #ifdef EHCI_STATS
>>>>>>           struct ehci_stats       stats;
>>>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>>>> +#      define COUNT(x) ((x)++)
>>>>>>     #else
>>>>>> -#      define COUNT(x) do {} while (0)
>>>>>> +#      define COUNT(x) ((void) 0)
>>>>>
>>>>>
>>>>>
>>>>>      Why not just empty #define?
>>>>
>>>>
>>>> Indeed. I'll change it.
>>>> Tks Sergei.
>>>
>>>
>>> Since COUNT is not used to return the empty #define is ok. Another way
>>> is to use #define COUNT(x) (0) to get a 0 when necessary to read
>>> returns.

>>     Just 0, no parens please.

> Ok, no parens, since there's no evaluation.

    It's because the literals don't need parens at all.

> Then my change is:
>
> -#      define COUNT(x) do { (x)++; } while (0)
> +#      define COUNT(x) (++(x))
>   #else
> -#      define COUNT(x) do {} while (0)
> +#      define COUNT(x) 0
>
> Pre-increment allowing to return the updated x.

    Why if there was a post-increment before?

    Anyway, this talk is quite pointless since the macro didn't return any 
value anyway.

MBR, Sergei

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

2015-12-10 14:26 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> On 12/10/2015 05:56 PM, Geyslan G. Bem wrote:
>
>>>>>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>>>>>
>>>>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>>>>> use a do {} while (0) loop"
>>>>>>>
>>>>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>>>>> ---
>>>>>>>     drivers/usb/host/ehci.h | 4 ++--
>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>>>>> index cfeebd8..945000a 100644
>>>>>>> --- a/drivers/usb/host/ehci.h
>>>>>>> +++ b/drivers/usb/host/ehci.h
>>>>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>>>>> controller */
>>>>>>>           /* irq statistics */
>>>>>>>     #ifdef EHCI_STATS
>>>>>>>           struct ehci_stats       stats;
>>>>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>>>>> +#      define COUNT(x) ((x)++)
>>>>>>>     #else
>>>>>>> -#      define COUNT(x) do {} while (0)
>>>>>>> +#      define COUNT(x) ((void) 0)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>      Why not just empty #define?
>>>>>
>>>>>
>>>>>
>>>>> Indeed. I'll change it.
>>>>> Tks Sergei.
>>>>
>>>>
>>>>
>>>> Since COUNT is not used to return the empty #define is ok. Another way
>>>> is to use #define COUNT(x) (0) to get a 0 when necessary to read
>>>> returns.
>
>
>>>     Just 0, no parens please.
>
>
>> Ok, no parens, since there's no evaluation.
>
>
>    It's because the literals don't need parens at all.
>
>> Then my change is:
>>
>> -#      define COUNT(x) do { (x)++; } while (0)
>> +#      define COUNT(x) (++(x))
>>   #else
>> -#      define COUNT(x) do {} while (0)
>> +#      define COUNT(x) 0
>>
>> Pre-increment allowing to return the updated x.
>
>
>    Why if there was a post-increment before?
There's nothing wrong with post-increment. The pre one would be
necessary if using return.

>
>    Anyway, this talk is quite pointless since the macro didn't return any
> value anyway.
You're sure, there's no use anywhere of the return of that macro indeed.

Sending v2 soon.

>
> MBR, Sergei
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Sergei Shtylyov]

On 12/10/2015 08:40 PM, Geyslan G. Bem wrote:

>>>>>>>> Don't use the 'do {} while (0)' wrapper in a single statement macro.
>>>>>>>>
>>>>>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>>>>>> use a do {} while (0) loop"
>>>>>>>>
>>>>>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>>>>>> ---
>>>>>>>>      drivers/usb/host/ehci.h | 4 ++--
>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>>>>>> index cfeebd8..945000a 100644
>>>>>>>> --- a/drivers/usb/host/ehci.h
>>>>>>>> +++ b/drivers/usb/host/ehci.h
>>>>>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>>>>>> controller */
>>>>>>>>            /* irq statistics */
>>>>>>>>      #ifdef EHCI_STATS
>>>>>>>>            struct ehci_stats       stats;
>>>>>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>>>>>> +#      define COUNT(x) ((x)++)
>>>>>>>>      #else
>>>>>>>> -#      define COUNT(x) do {} while (0)
>>>>>>>> +#      define COUNT(x) ((void) 0)
>>>>>>>
>>>>>>>       Why not just empty #define?
>>>>>>
>>>>>> Indeed. I'll change it.
>>>>>> Tks Sergei.
>>>>>
>>>>> Since COUNT is not used to return the empty #define is ok. Another way
>>>>> is to use #define COUNT(x) (0) to get a 0 when necessary to read
>>>>> returns.
>>
>>>>      Just 0, no parens please.
>>
>>> Ok, no parens, since there's no evaluation.
>>
>>     It's because the literals don't need parens at all.
>>
>>> Then my change is:
>>>
>>> -#      define COUNT(x) do { (x)++; } while (0)
>>> +#      define COUNT(x) (++(x))
>>>    #else
>>> -#      define COUNT(x) do {} while (0)
>>> +#      define COUNT(x) 0
>>>
>>> Pre-increment allowing to return the updated x.
>>
>>
>>     Why if there was a post-increment before?

> There's nothing wrong with post-increment. The pre one would be
> necessary if using return.

    Maybe it was intended to return the old value? :-)

>>
>>     Anyway, this talk is quite pointless since the macro didn't return any
>> value anyway.
> You're sure, there's no use anywhere of the return of that macro indeed.

    *do* {} *while* (0) just couldn't return any value, it's not just a 
compound statement which gcc indeed allows to be evaluated.

> Sending v2 soon.

MBR, Sergei

Re: [PATCH 4/9v2] usb: host: ehci.h: fix single statement macros

[Geyslan G. Bem]

2015-12-10 14:47 GMT-03:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> On 12/10/2015 08:40 PM, Geyslan G. Bem wrote:
>
>>>>>>>>> Don't use the 'do {} while (0)' wrapper in a single statement
>>>>>>>>> macro.
>>>>>>>>>
>>>>>>>>> Caught by checkpatch: "WARNING: Single statement macros should not
>>>>>>>>> use a do {} while (0) loop"
>>>>>>>>>
>>>>>>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>>>>>>> ---
>>>>>>>>>      drivers/usb/host/ehci.h | 4 ++--
>>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
>>>>>>>>> index cfeebd8..945000a 100644
>>>>>>>>> --- a/drivers/usb/host/ehci.h
>>>>>>>>> +++ b/drivers/usb/host/ehci.h
>>>>>>>>> @@ -244,9 +244,9 @@ struct ehci_hcd {                   /* one per
>>>>>>>>> controller */
>>>>>>>>>            /* irq statistics */
>>>>>>>>>      #ifdef EHCI_STATS
>>>>>>>>>            struct ehci_stats       stats;
>>>>>>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>>>>>>> +#      define COUNT(x) ((x)++)
>>>>>>>>>      #else
>>>>>>>>> -#      define COUNT(x) do {} while (0)
>>>>>>>>> +#      define COUNT(x) ((void) 0)
>>>>>>>>
>>>>>>>>
>>>>>>>>       Why not just empty #define?
>>>>>>>
>>>>>>>
>>>>>>> Indeed. I'll change it.
>>>>>>> Tks Sergei.
>>>>>>
>>>>>>
>>>>>> Since COUNT is not used to return the empty #define is ok. Another way
>>>>>> is to use #define COUNT(x) (0) to get a 0 when necessary to read
>>>>>> returns.
>>>
>>>
>>>>>      Just 0, no parens please.
>>>
>>>
>>>> Ok, no parens, since there's no evaluation.
>>>
>>>
>>>     It's because the literals don't need parens at all.
>>>
>>>> Then my change is:
>>>>
>>>> -#      define COUNT(x) do { (x)++; } while (0)
>>>> +#      define COUNT(x) (++(x))
>>>>    #else
>>>> -#      define COUNT(x) do {} while (0)
>>>> +#      define COUNT(x) 0
>>>>
>>>> Pre-increment allowing to return the updated x.
>>>
>>>
>>>
>>>     Why if there was a post-increment before?
>
>
>> There's nothing wrong with post-increment. The pre one would be
>> necessary if using return.
>
>
>    Maybe it was intended to return the old value? :-)
>
>>>
>>>     Anyway, this talk is quite pointless since the macro didn't return
>>> any
>>> value anyway.
>>
>> You're sure, there's no use anywhere of the return of that macro indeed.
>
>
>    *do* {} *while* (0) just couldn't return any value, it's not just a
> compound statement which gcc indeed allows to be evaluated.
Indeed. :-)

v2 in the oven.
>
>
>> Sending v2 soon.
>
>
> MBR, Sergei
>



-- 
Regards,

Geyslan G. Bem
hackingbits.com