Date:	Wed, 10 Dec 2008 10:42:17 +0100
From:	Laurent Pinchart <laurent.pinchart@...net.be>
To:	Wu Fengguang <fengguang.wu@...el.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...e.de>, linux-usb@...r.kernel.org
Subject: Re: [PATCH] usb: make printk messges more searchable

Hi Wu,

On Wednesday 10 December 2008, Wu Fengguang wrote:
> Make USB printk messages long and straightforward.  One of these
> decorated USB error messages cost me some smart efforts to locate.

That would make the code break the 80 columns limit.

>From "Documentation/CodingStyle":

"The limit on the length of lines is 80 columns and this is a strongly
preferred limit."

Best regards,

Laurent Pinchart

> Cc: Greg Kroah-Hartman <gregkh@...e.de>
> Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> ---
>  drivers/usb/core/hub.c |   89 ++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 46 deletions(-)
>
> --- linux-2.6.orig/drivers/usb/core/hub.c
> +++ linux-2.6/drivers/usb/core/hub.c
> @@ -107,7 +107,8 @@ MODULE_PARM_DESC (blinkenlights, "true t
>  /* define initial 64-byte descriptor request timeout in milliseconds */
>  static int initial_descriptor_timeout = USB_CTRL_GET_TIMEOUT;
>  module_param(initial_descriptor_timeout, int, S_IRUGO|S_IWUSR);
> -MODULE_PARM_DESC(initial_descriptor_timeout, "initial 64-byte descriptor
> request timeout in milliseconds (default 5000 - 5.0 seconds)");
> +MODULE_PARM_DESC(initial_descriptor_timeout,
> +		"initial 64-byte descriptor request timeout in milliseconds (default
> 5000 - 5.0 seconds)");
>
>  /*
>   * As of 2.6.10 we introduce a new USB device initialization scheme which
> @@ -131,8 +132,7 @@ MODULE_PARM_DESC(old_scheme_first,
>  static int use_both_schemes = 1;
>  module_param(use_both_schemes, bool, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(use_both_schemes,
> -		"try the other device initialization scheme if the "
> -		"first one fails");
> +		"try the other device initialization scheme if the first one fails");
>
>  /* Mutual exclusion for EHCI CF initialization.  This interferes with
>   * port reset on some companion controllers.
> @@ -545,8 +545,7 @@ static unsigned hub_power_on(struct usb_
>  	if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
>  		dev_dbg(hub->intfdev, "enabling power on all ports\n");
>  	else
> -		dev_dbg(hub->intfdev, "trying to enable port power on "
> -				"non-switchable hub\n");
> +		dev_dbg(hub->intfdev, "trying to enable port power on non-switchable
> hub\n"); for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
>  		set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
>
> @@ -1020,8 +1019,7 @@ static int hub_configure(struct usb_hub
>
>  			if (remaining < hdev->maxchild * 100)
>  				dev_warn(hub_dev,
> -					"insufficient power available "
> -					"to use all downstream ports\n");
> +					"insufficient power available to use all downstream ports\n");
>  			hub->mA_per_port = 100;		/* 7.2.1.1 */
>  		}
>  	} else {	/* Self-powered external hub */
> @@ -1136,8 +1134,8 @@ static int hub_probe(struct usb_interfac
>  	hdev = interface_to_usbdev(intf);
>
>  	if (hdev->level == MAX_TOPO_LEVEL) {
> -		dev_err(&intf->dev, "Unsupported bus topology: "
> -				"hub nested too deep\n");
> +		dev_err(&intf->dev,
> +			"Unsupported bus topology: hub nested too deep\n");
>  		return -E2BIG;
>  	}
>
> @@ -1476,8 +1474,8 @@ static void announce_device(struct usb_d
>  	dev_info(&udev->dev, "New USB device found, idVendor=%04x,
> idProduct=%04x\n", le16_to_cpu(udev->descriptor.idVendor),
>  		le16_to_cpu(udev->descriptor.idProduct));
> -	dev_info(&udev->dev, "New USB device strings: Mfr=%d, Product=%d, "
> -		"SerialNumber=%d\n",
> +	dev_info(&udev->dev,
> +		"New USB device strings: Mfr=%d, Product=%d, SerialNumber=%d\n",
>  		udev->descriptor.iManufacturer,
>  		udev->descriptor.iProduct,
>  		udev->descriptor.iSerialNumber);
> @@ -1542,7 +1540,7 @@ static int usb_configure_device_otg(stru
>  					 * customize to match your product.
>  					 */
>  					dev_info(&udev->dev,
> -						"can't set HNP mode; %d\n",
> +						"can't set HNP mode: %d\n",
>  						err);
>  					bus->b_hnp_enable = 0;
>  				}
> @@ -1723,8 +1721,9 @@ int usb_authorize_device(struct usb_devi
>  	}
>  	result = usb_get_device_descriptor(usb_dev, sizeof(usb_dev->descriptor));
>  	if (result < 0) {
> -		dev_err(&usb_dev->dev, "can't re-read device descriptor for "
> -			"authorization: %d\n", result);
> +		dev_err(&usb_dev->dev,
> +			"can't re-read device descriptor for authorization: %d\n",
> +			result);
>  		goto error_device_descriptor;
>  	}
>  	usb_dev->authorized = 1;
> @@ -2020,8 +2019,8 @@ int usb_port_suspend(struct usb_device *
>  				USB_CTRL_SET_TIMEOUT);
>  	} else {
>  		/* device has up to 10 msec to fully suspend */
> -		dev_dbg(&udev->dev, "usb %ssuspend\n",
> -				udev->auto_pm ? "auto-" : "");
> +		dev_dbg(&udev->dev, "%s\n",
> +			udev->auto_pm ? "usb auto-suspend" : "usb suspend");
>  		usb_set_device_state(udev, USB_STATE_SUSPENDED);
>  		msleep(10);
>  	}
> @@ -2045,8 +2044,8 @@ static int finish_port_resume(struct usb
>  	u16	devstatus;
>
>  	/* caller owns the udev device lock */
> -	dev_dbg(&udev->dev, "finish %sresume\n",
> -			udev->reset_resume ? "reset-" : "");
> +	dev_dbg(&udev->dev, "%s\n",
> +		udev->reset_resume ? "finish reset-resume" : "finish resume");
>
>  	/* usb ch9 identifies four variants of SUSPENDED, based on what
>  	 * state the device resumes to.  Linux currently won't see the
> @@ -2098,8 +2097,9 @@ static int finish_port_resume(struct usb
>  					NULL, 0,
>  					USB_CTRL_SET_TIMEOUT);
>  			if (status)
> -				dev_dbg(&udev->dev, "disable remote "
> -					"wakeup, status %d\n", status);
> +				dev_dbg(&udev->dev,
> +					"disable remote wakeup, status %d\n",
> +					status);
>  		}
>  		status = 0;
>  	}
> @@ -2582,9 +2582,9 @@ hub_port_init (struct usb_hub *hub, stru
>  				goto fail;
>  			}
>  			if (r) {
> -				dev_err(&udev->dev, "device descriptor "
> -						"read/%s, error %d\n",
> -						"64", r);
> +				dev_err(&udev->dev,
> +					"device descriptor read/64, error %d\n",
> +					r);
>  				retval = -EMSGSIZE;
>  				continue;
>  			}
> @@ -2621,9 +2621,9 @@ hub_port_init (struct usb_hub *hub, stru
>
>  		retval = usb_get_device_descriptor(udev, 8);
>  		if (retval < 8) {
> -			dev_err(&udev->dev, "device descriptor "
> -					"read/%s, error %d\n",
> -					"8", retval);
> +			dev_err(&udev->dev,
> +					"device descriptor read/8, error %d\n",
> +					retval);
>  			if (retval >= 0)
>  				retval = -EMSGSIZE;
>  		} else {
> @@ -2650,8 +2650,8 @@ hub_port_init (struct usb_hub *hub, stru
>
>  	retval = usb_get_device_descriptor(udev, USB_DT_DEVICE_SIZE);
>  	if (retval < (signed)sizeof(udev->descriptor)) {
> -		dev_err(&udev->dev, "device descriptor read/%s, error %d\n",
> -			"all", retval);
> +		dev_err(&udev->dev, "device descriptor read/all, error %d\n",
> +			retval);
>  		if (retval >= 0)
>  			retval = -ENOMSG;
>  		goto fail;
> @@ -2681,8 +2681,8 @@ check_highspeed (struct usb_hub *hub, st
>  	status = usb_get_descriptor (udev, USB_DT_DEVICE_QUALIFIER, 0,
>  			qual, sizeof *qual);
>  	if (status == sizeof *qual) {
> -		dev_info(&udev->dev, "not running at top speed; "
> -			"connect to a high speed hub\n");
> +		dev_info(&udev->dev,
> +			"not running at top speed; connect to a high speed hub\n");
>  		/* hub LEDs are probably harder to miss than syslog */
>  		if (hub->has_indicators) {
>  			hub->indicator[port1-1] = INDICATOR_GREEN_BLINK;
> @@ -2719,9 +2719,9 @@ hub_power_remaining (struct usb_hub *hub
>  		else
>  			delta = 8;
>  		if (delta > hub->mA_per_port)
> -			dev_warn(&udev->dev, "%dmA is over %umA budget "
> -					"for port %d!\n",
> -					delta, hub->mA_per_port, port1);
> +			dev_warn(&udev->dev,
> +				 "%dmA is over %umA budget for port %d!\n",
> +				 delta, hub->mA_per_port, port1);
>  		remaining -= delta;
>  	}
>  	if (remaining < 0) {
> @@ -2812,8 +2812,9 @@ static void hub_port_connect_change(stru
>  		status = hub_port_debounce(hub, port1);
>  		if (status < 0) {
>  			if (printk_ratelimit())
> -				dev_err(hub_dev, "connect-debounce failed, "
> -						"port %d disabled\n", port1);
> +				dev_err(hub_dev,
> +					"connect-debounce failed, port %d disabled\n",
> +					port1);
>  			portstatus &= ~USB_PORT_STAT_CONNECTION;
>  		} else {
>  			portstatus = status;
> @@ -2883,8 +2884,7 @@ static void hub_port_connect_change(stru
>  			le16_to_cpus(&devstat);
>  			if ((devstat & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
>  				dev_err(&udev->dev,
> -					"can't connect bus-powered hub "
> -					"to this port\n");
> +					"can't connect bus-powered hub to this port\n");
>  				if (hub->has_indicators) {
>  					hub->indicator[port1-1] =
>  						INDICATOR_AMBER_BLINK;
> @@ -3067,9 +3067,8 @@ static void hub_events(void)
>  			if (portchange & USB_PORT_STAT_C_ENABLE) {
>  				if (!connect_change)
>  					dev_dbg (hub_dev,
> -						"port %d enable change, "
> -						"status %08x\n",
> -						i, portstatus);
> +						 "port %d enable change, status %08x\n",
> +						 i, portstatus);
>  				clear_port_feature(hdev, i,
>  					USB_PORT_FEAT_C_ENABLE);
>
> @@ -3083,10 +3082,8 @@ static void hub_events(void)
>  				    && !connect_change
>  				    && hdev->children[i-1]) {
>  					dev_err (hub_dev,
> -					    "port %i "
> -					    "disabled by hub (EMI?), "
> -					    "re-enabling...\n",
> -						i);
> +						 "port %i disabled by hub (EMI?), re-enabling...\n",
> +						 i);
>  					connect_change = 1;
>  				}
>  			}
> @@ -3420,8 +3417,8 @@ static int usb_reset_and_verify_device(s
>  		ret = usb_set_interface(udev, desc->bInterfaceNumber,
>  			desc->bAlternateSetting);
>  		if (ret < 0) {
> -			dev_err(&udev->dev, "failed to restore interface %d "
> -				"altsetting %d (error=%d)\n",
> +			dev_err(&udev->dev,
> +				"failed to restore interface %d altsetting %d (error=%d)\n",
>  				desc->bInterfaceNumber,
>  				desc->bAlternateSetting,
>  				ret);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists