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

[permalink] [raw]

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

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 <[email protected]>
---
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

2015-12-09 22:33:18

[permalink] [raw]

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

Get rid of space before open square bracket.

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

Signed-off-by: Geyslan G. Bem <[email protected]>
---
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

2015-12-09 22:33:46

[permalink] [raw]

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

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 <[email protected]>
---
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

2015-12-09 22:33:52

[permalink] [raw]

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

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

Caught by checkpatch: "WARNING"

Signed-off-by: Geyslan G. Bem <[email protected]>
---
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

2015-12-09 22:33:55

[permalink] [raw]

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

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 <[email protected]>
---
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

2015-12-09 22:33:59

[permalink] [raw]

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

Removes trailing semicolon from macros.

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

Signed-off-by: Geyslan G. Bem <[email protected]>
---
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

2015-12-09 22:34:05

[permalink] [raw]

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

The pointer operator must be sticked to name.

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

Signed-off-by: Geyslan G. Bem <[email protected]>
---
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

2015-12-10 09:15:48

[permalink] [raw]

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

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 <[email protected]>
> ---
It is a good idea to include here (after the ---) what changed from V1.
Other than that:
Acked-by: Peter Senna Tschudin <[email protected]>

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
>

2015-12-10 09:30:49

[permalink] [raw]

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

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 <[email protected]>
> ---
As with previous, missing the changelog here, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> 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
>

2015-12-10 09:27:55

[permalink] [raw]

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

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 <[email protected]>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> ---
> 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
>

2015-12-10 09:58:29

[permalink] [raw]

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

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 <[email protected]>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> ---
> 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
>

2015-12-10 10:02:02

[permalink] [raw]

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

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 <[email protected]>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> ---
> 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
>

2015-12-10 10:19:05

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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
>

2015-12-10 10:20:20

[permalink] [raw]

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

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 <[email protected]>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> ---
> 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
>

2015-12-10 10:21:59

[permalink] [raw]

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

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 <[email protected]>
As with previous, missing the changelog, but other than that the patch
looks good to me.

Acked-by: Peter Senna Tschudin <[email protected]>
> ---
> 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
>

2015-12-10 10:25:18

[permalink] [raw]

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

2015-12-10 7:01 GMT-03:00 Peter Senna Tschudin <[email protected]>:
> 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 <[email protected]>
> As with previous, missing the changelog, but other than that the patch
> looks good to me.
>
> Acked-by: Peter Senna Tschudin <[email protected]>
>> ---
>> 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

2015-12-10 11:55:03

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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

2015-12-10 12:20:16

[permalink] [raw]

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

2015-12-10 8:54 GMT-03:00 Sergei Shtylyov <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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

2015-12-10 13:05:23

[permalink] [raw]

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

2015-12-10 9:20 GMT-03:00 Geyslan G. Bem <[email protected]>:
> 2015-12-10 8:54 GMT-03:00 Sergei Shtylyov <[email protected]>:
>> 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 <[email protected]>
>>> ---
>>> 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

2015-12-10 14:44:10

[permalink] [raw]

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

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 <[email protected]>
>>>> ---
>>>> 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

2015-12-10 14:56:38

[permalink] [raw]

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

2015-12-10 11:44 GMT-03:00 Sergei Shtylyov <[email protected]>:
> 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 <[email protected]>
>>>>> ---
>>>>> 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

2015-12-10 15:57:25

by Alan Stern

[permalink] [raw]

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

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

2015-12-10 16:02:18

[permalink] [raw]

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

2015-12-10 12:57 GMT-03:00 Alan Stern <[email protected]>:
> 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

2015-12-10 17:26:27

[permalink] [raw]

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

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 <[email protected]>
>>>>>> ---
>>>>>> 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

2015-12-10 17:40:42

[permalink] [raw]

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

2015-12-10 14:26 GMT-03:00 Sergei Shtylyov <[email protected]>:
> 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 <[email protected]>
>>>>>>> ---
>>>>>>> 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

2015-12-10 17:47:14

[permalink] [raw]

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

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 <[email protected]>
>>>>>>>> ---
>>>>>>>> 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

2015-12-10 18:04:12