2013-10-05 16:11:20

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 0/5] Cleanup in drivers/usb/core/hcd.c

Hi!

I did some cleanup in

drivers/usb/core/hcd.c

of ERROR messages from "scripts/checkpatch.pl --file". Not all ERRORs are fixed,
there are some remaining with "assignment in if statement".

It's based on 7dee8df, which is Linus current master. I compiled it without
errors. Checkpatch script gives me some warnings, which are remaining warnings
of the file, I didn't introduce them!

Best regards!

Matthias Beyer (5):
drivers: usb: core: hcd: moved asterix to variable
drivers: usb: core: hcd: Whitespace fixes
drivers: usb: core: hcd: replaced C99 // comments
drivers: usb: core: hcd: removed braces for return statements
drivers: usb: core: hcd: if-else-braces fixed

drivers/usb/core/hcd.c | 80 +++++++++++++++++++++++++-------------------------
1 file changed, 40 insertions(+), 40 deletions(-)

--
1.8.4


2013-10-05 16:11:15

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 1/5] drivers: usb: core: hcd: moved asterix to variable

instead of type

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index d6a8d23..9036794 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2818,7 +2818,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
EXPORT_SYMBOL_GPL(usb_remove_hcd);

void
-usb_hcd_platform_shutdown(struct platform_device* dev)
+usb_hcd_platform_shutdown(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);

--
1.8.4

2013-10-05 16:11:22

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 2/5] drivers: usb: core: hcd: Whitespace fixes

including
- spaces to tabs
- removing spaces before array indexing (foo [] to foo[])
- adding spaces around unary operator (foo? 1 : 0 to foo ? 1 : 0)
- removed trailing whitespace

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 56 +++++++++++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9036794..0f3e5a0 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -6,7 +6,7 @@
* (C) Copyright Deti Fliegl 1999
* (C) Copyright Randy Dunlap 2000
* (C) Copyright David Brownell 2000-2002
- *
+ *
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
@@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
/* used when allocating bus numbers */
#define USB_MAXBUS 64
struct usb_busmap {
- unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
+ unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
};
static struct usb_busmap busmap;

@@ -171,7 +171,7 @@ static const u8 usb25_rh_dev_descriptor[18] = {
};

/* usb 2.0 root hub device descriptor */
-static const u8 usb2_rh_dev_descriptor [18] = {
+static const u8 usb2_rh_dev_descriptor[18] = {
0x12, /* __u8 bLength; */
0x01, /* __u8 bDescriptorType; Device */
0x00, 0x02, /* __le16 bcdUSB; v2.0 */
@@ -194,7 +194,7 @@ static const u8 usb2_rh_dev_descriptor [18] = {
/* no usb 2.0 root hub "device qualifier" descriptor: one speed only */

/* usb 1.1 root hub device descriptor */
-static const u8 usb11_rh_dev_descriptor [18] = {
+static const u8 usb11_rh_dev_descriptor[18] = {
0x12, /* __u8 bLength; */
0x01, /* __u8 bDescriptorType; Device */
0x10, 0x01, /* __le16 bcdUSB; v1.1 */
@@ -219,7 +219,7 @@ static const u8 usb11_rh_dev_descriptor [18] = {

/* Configuration descriptors for our root hubs */

-static const u8 fs_rh_config_descriptor [] = {
+static const u8 fs_rh_config_descriptor[] = {

/* one configuration */
0x09, /* __u8 bLength; */
@@ -228,13 +228,13 @@ static const u8 fs_rh_config_descriptor [] = {
0x01, /* __u8 bNumInterfaces; (1) */
0x01, /* __u8 bConfigurationValue; */
0x00, /* __u8 iConfiguration; */
- 0xc0, /* __u8 bmAttributes;
+ 0xc0, /* __u8 bmAttributes;
Bit 7: must be set,
6: Self-powered,
5: Remote wakeup,
4..0: resvd */
0x00, /* __u8 MaxPower; */
-
+
/* USB 1.1:
* USB 2.0, single TT organization (mandatory):
* one interface, protocol 0
@@ -256,17 +256,17 @@ static const u8 fs_rh_config_descriptor [] = {
0x00, /* __u8 if_bInterfaceSubClass; */
0x00, /* __u8 if_bInterfaceProtocol; [usb1.1 or single tt] */
0x00, /* __u8 if_iInterface; */
-
+
/* one endpoint (status change endpoint) */
0x07, /* __u8 ep_bLength; */
0x05, /* __u8 ep_bDescriptorType; Endpoint */
0x81, /* __u8 ep_bEndpointAddress; IN Endpoint 1 */
- 0x03, /* __u8 ep_bmAttributes; Interrupt */
- 0x02, 0x00, /* __le16 ep_wMaxPacketSize; 1 + (MAX_ROOT_PORTS / 8) */
+ 0x03, /* __u8 ep_bmAttributes; Interrupt */
+ 0x02, 0x00, /* __le16 ep_wMaxPacketSize; 1 + (MAX_ROOT_PORTS / 8) */
0xff /* __u8 ep_bInterval; (255ms -- usb 2.0 spec) */
};

-static const u8 hs_rh_config_descriptor [] = {
+static const u8 hs_rh_config_descriptor[] = {

/* one configuration */
0x09, /* __u8 bLength; */
@@ -275,13 +275,13 @@ static const u8 hs_rh_config_descriptor [] = {
0x01, /* __u8 bNumInterfaces; (1) */
0x01, /* __u8 bConfigurationValue; */
0x00, /* __u8 iConfiguration; */
- 0xc0, /* __u8 bmAttributes;
+ 0xc0, /* __u8 bmAttributes;
Bit 7: must be set,
6: Self-powered,
5: Remote wakeup,
4..0: resvd */
0x00, /* __u8 MaxPower; */
-
+
/* USB 1.1:
* USB 2.0, single TT organization (mandatory):
* one interface, protocol 0
@@ -303,12 +303,12 @@ static const u8 hs_rh_config_descriptor [] = {
0x00, /* __u8 if_bInterfaceSubClass; */
0x00, /* __u8 if_bInterfaceProtocol; [usb1.1 or single tt] */
0x00, /* __u8 if_iInterface; */
-
+
/* one endpoint (status change endpoint) */
0x07, /* __u8 ep_bLength; */
0x05, /* __u8 ep_bDescriptorType; Endpoint */
0x81, /* __u8 ep_bEndpointAddress; IN Endpoint 1 */
- 0x03, /* __u8 ep_bmAttributes; Interrupt */
+ 0x03, /* __u8 ep_bmAttributes; Interrupt */
/* __le16 ep_wMaxPacketSize; 1 + (MAX_ROOT_PORTS / 8)
* see hub.c:hub_configure() for details. */
(USB_MAXCHILDREN + 1 + 7) / 8, 0x00,
@@ -464,7 +464,7 @@ rh_string(int id, struct usb_hcd const *hcd, u8 *data, unsigned len)
static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
{
struct usb_ctrlrequest *cmd;
- u16 typeReq, wValue, wIndex, wLength;
+ u16 typeReq, wValue, wIndex, wLength;
u8 *ubuf = urb->transfer_buffer;
unsigned len = 0;
int status;
@@ -526,10 +526,10 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
*/

case DeviceRequest | USB_REQ_GET_STATUS:
- tbuf [0] = (device_may_wakeup(&hcd->self.root_hub->dev)
+ tbuf[0] = (device_may_wakeup(&hcd->self.root_hub->dev)
<< USB_DEVICE_REMOTE_WAKEUP)
| (1 << USB_DEVICE_SELF_POWERED);
- tbuf [1] = 0;
+ tbuf[1] = 0;
len = 2;
break;
case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
@@ -546,7 +546,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
goto error;
break;
case DeviceRequest | USB_REQ_GET_CONFIGURATION:
- tbuf [0] = 1;
+ tbuf[0] = 1;
len = 1;
/* FALLTHROUGH */
case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
@@ -609,7 +609,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
}
break;
case DeviceRequest | USB_REQ_GET_INTERFACE:
- tbuf [0] = 0;
+ tbuf[0] = 0;
len = 1;
/* FALLTHROUGH */
case DeviceOutRequest | USB_REQ_SET_INTERFACE:
@@ -626,8 +626,8 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)

case EndpointRequest | USB_REQ_GET_STATUS:
// ENDPOINT_HALT flag
- tbuf [0] = 0;
- tbuf [1] = 0;
+ tbuf[0] = 0;
+ tbuf[1] = 0;
len = 2;
/* FALLTHROUGH */
case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
@@ -877,7 +877,7 @@ static ssize_t authorized_default_store(struct device *dev,
usb_hcd = bus_to_hcd(usb_bus);
result = sscanf(buf, "%u\n", &val);
if (result == 1) {
- usb_hcd->authorized_default = val? 1 : 0;
+ usb_hcd->authorized_default = val ? 1 : 0;
result = size;
}
else
@@ -1812,7 +1812,7 @@ rescan:
case USB_ENDPOINT_XFER_INT:
s = "-intr"; break;
default:
- s = "-iso"; break;
+ s = "-iso"; break;
};
s;
}));
@@ -2245,7 +2245,7 @@ static void hcd_resume_work(struct work_struct *work)
}

/**
- * usb_hcd_resume_root_hub - called by HCD to resume its root hub
+ * usb_hcd_resume_root_hub - called by HCD to resume its root hub
* @hcd: host controller for this root hub
*
* The USB host controller calls this function when its root hub is
@@ -2600,7 +2600,7 @@ int usb_add_hcd(struct usb_hcd *hcd,

/* Keep old behaviour if authorized_default is not in [0, 1]. */
if (authorized_default < 0 || authorized_default > 1)
- hcd->authorized_default = hcd->wireless? 0 : 1;
+ hcd->authorized_default = hcd->wireless ? 0 : 1;
else
hcd->authorized_default = authorized_default;
set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
@@ -2743,7 +2743,7 @@ err_allocate_root_hub:
err_register_bus:
hcd_buffer_destroy(hcd);
return retval;
-}
+}
EXPORT_SYMBOL_GPL(usb_add_hcd);

/**
@@ -2840,7 +2840,7 @@ struct usb_mon_operations *mon_ops;
* Notice that the code is minimally error-proof. Because usbmon needs
* symbols from usbcore, usbcore gets referenced and cannot be unloaded first.
*/
-
+
int usb_mon_register (struct usb_mon_operations *ops)
{

--
1.8.4

2013-10-05 16:11:33

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 5/5] drivers: usb: core: hcd: if-else-braces fixed

Put else keyword on same line as closing brace from if statement, added
{ } braces as the styleguide says.

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 5d13d6c..bbd182a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -879,9 +879,9 @@ static ssize_t authorized_default_store(struct device *dev,
if (result == 1) {
usb_hcd->authorized_default = val ? 1 : 0;
result = size;
- }
- else
+ } else {
result = -EINVAL;
+ }
return result;
}
static DEVICE_ATTR_RW(authorized_default);
--
1.8.4

2013-10-05 16:11:26

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 3/5] drivers: usb: core: hcd: replaced C99 // comments

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 0f3e5a0..3a2667c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -428,7 +428,7 @@ rh_string(int id, struct usb_hcd const *hcd, u8 *data, unsigned len)
char const *s;
static char const langids[4] = {4, USB_DT_STRING, 0x09, 0x04};

- // language ids
+ /* language ids */
switch (id) {
case 0:
/* Array of LANGID codes (0x0409 is MSFT-speak for "en-us") */
@@ -615,7 +615,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
case DeviceOutRequest | USB_REQ_SET_INTERFACE:
break;
case DeviceOutRequest | USB_REQ_SET_ADDRESS:
- // wValue == urb->dev->devaddr
+ /* wValue == urb->dev->devaddr */
dev_dbg (hcd->self.controller, "root hub device address %d\n",
wValue);
break;
@@ -625,7 +625,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
/* ENDPOINT REQUESTS */

case EndpointRequest | USB_REQ_GET_STATUS:
- // ENDPOINT_HALT flag
+ /* ENDPOINT_HALT flag */
tbuf[0] = 0;
tbuf[1] = 0;
len = 2;
@@ -683,7 +683,7 @@ error:
if (urb->transfer_buffer_length < len)
len = urb->transfer_buffer_length;
urb->actual_length = len;
- // always USB_DIR_IN, toward host
+ /* always USB_DIR_IN, toward host */
memcpy (ubuf, bufp, len);

/* report whether RH hardware supports remote wakeup */
@@ -1134,7 +1134,7 @@ long usb_calc_bus_time (int speed, int is_input, int isoc, int bytecount)
return (9107L + BW_HOST_DELAY + tmp);
}
case USB_SPEED_HIGH: /* ISOC or INTR */
- // FIXME adjust for input vs output
+ /* FIXME adjust for input vs output */
if (isoc)
tmp = HS_NSECS_ISO (bytecount);
else
--
1.8.4

2013-10-05 16:11:47

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 4/5] drivers: usb: core: hcd: removed braces for return statements

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 3a2667c..5d13d6c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1120,18 +1120,18 @@ long usb_calc_bus_time (int speed, int is_input, int isoc, int bytecount)
case USB_SPEED_LOW: /* INTR only */
if (is_input) {
tmp = (67667L * (31L + 10L * BitTime (bytecount))) / 1000L;
- return (64060L + (2 * BW_HUB_LS_SETUP) + BW_HOST_DELAY + tmp);
+ return 64060L + (2 * BW_HUB_LS_SETUP) + BW_HOST_DELAY + tmp;
} else {
tmp = (66700L * (31L + 10L * BitTime (bytecount))) / 1000L;
- return (64107L + (2 * BW_HUB_LS_SETUP) + BW_HOST_DELAY + tmp);
+ return 64107L + (2 * BW_HUB_LS_SETUP) + BW_HOST_DELAY + tmp;
}
case USB_SPEED_FULL: /* ISOC or INTR */
if (isoc) {
tmp = (8354L * (31L + 10L * BitTime (bytecount))) / 1000L;
- return (((is_input) ? 7268L : 6265L) + BW_HOST_DELAY + tmp);
+ return ((is_input) ? 7268L : 6265L) + BW_HOST_DELAY + tmp;
} else {
tmp = (8354L * (31L + 10L * BitTime (bytecount))) / 1000L;
- return (9107L + BW_HOST_DELAY + tmp);
+ return 9107L + BW_HOST_DELAY + tmp;
}
case USB_SPEED_HIGH: /* ISOC or INTR */
/* FIXME adjust for input vs output */
--
1.8.4

2013-10-05 18:04:09

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/5] drivers: usb: core: hcd: Whitespace fixes

On Sat, 2013-10-05 at 18:02 +0200, Matthias Beyer wrote:
> - removing spaces before array indexing (foo [] to foo[])
[]
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
[]
> @@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
> /* used when allocating bus numbers */
> #define USB_MAXBUS 64
> struct usb_busmap {
> - unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
> + unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];

this would be nicer as

DEFINE_BITMAP(busmap, USB_MAXBUS);

2013-10-05 18:05:23

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/5] drivers: usb: core: hcd: Whitespace fixes

On Sat, 2013-10-05 at 11:04 -0700, Joe Perches wrote:
> On Sat, 2013-10-05 at 18:02 +0200, Matthias Beyer wrote:
> > - removing spaces before array indexing (foo [] to foo[])
> []
> > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> []
> > @@ -93,7 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);
> > /* used when allocating bus numbers */
> > #define USB_MAXBUS 64
> > struct usb_busmap {
> > - unsigned long busmap [USB_MAXBUS / (8*sizeof (unsigned long))];
> > + unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
>
> this would be nicer as
>
> DEFINE_BITMAP(busmap, USB_MAXBUS);
>

nack, DECLARE_BITMAP(...)

2013-10-05 21:11:51

by Matthias Beyer

[permalink] [raw]
Subject: [PATCH 1/1] drivers: usb: core: hcd.c: converted busmap from struct to bitmap

The DECLARE_BITMAP macro should be used for declaring this bitmap.
This commit converts the busmap from a struct to a simple (static)
bitmap, using the DECLARE_BITMAP macro from linux/types.h.

Please review, as I'm new to kernel development, I don't know if this
has any hidden side effects!

Suggested by [email protected]

Signed-off-by: Matthias Beyer <[email protected]>
---
drivers/usb/core/hcd.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index bbd182a..e9e1b33 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -40,6 +40,7 @@
#include <linux/platform_device.h>
#include <linux/workqueue.h>
#include <linux/pm_runtime.h>
+#include <linux/types.h>

#include <linux/usb.h>
#include <linux/usb/hcd.h>
@@ -92,10 +93,7 @@ EXPORT_SYMBOL_GPL (usb_bus_list);

/* used when allocating bus numbers */
#define USB_MAXBUS 64
-struct usb_busmap {
- unsigned long busmap[USB_MAXBUS / (8*sizeof (unsigned long))];
-};
-static struct usb_busmap busmap;
+static DECLARE_BITMAP(busmap, USB_MAXBUS);

/* used when updating list of hcds */
DEFINE_MUTEX(usb_bus_list_lock); /* exported only for usbfs */
@@ -941,12 +939,12 @@ static int usb_register_bus(struct usb_bus *bus)
int busnum;

mutex_lock(&usb_bus_list_lock);
- busnum = find_next_zero_bit (busmap.busmap, USB_MAXBUS, 1);
+ busnum = find_next_zero_bit(busmap, USB_MAXBUS, 1);
if (busnum >= USB_MAXBUS) {
printk (KERN_ERR "%s: too many buses\n", usbcore_name);
goto error_find_busnum;
}
- set_bit (busnum, busmap.busmap);
+ set_bit(busnum, busmap);
bus->busnum = busnum;

/* Add it to the local list of buses */
@@ -987,7 +985,7 @@ static void usb_deregister_bus (struct usb_bus *bus)

usb_notify_remove_bus(bus);

- clear_bit (bus->busnum, busmap.busmap);
+ clear_bit(bus->busnum, busmap);
}

/**
--
1.8.4

2013-10-06 20:52:19

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 1/5] drivers: usb: core: hcd: moved asterix to variable

Hello.

On 05-10-2013 18:02, Matthias Beyer wrote:

s/asterix/asterisk/. Asterix is a hero of the infamous French movies,
asterisk is *.

> instead of type

Don't continue the subject this way in he changelog.

> Signed-off-by: Matthias Beyer <[email protected]>

WBR, Sergei