Re: [PATCH] Staging: wlan-ng: fix many style warnings in hfa384x_usb.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2010-04-22 at 17:49 +0200, Alessandro Ghedini wrote:
> This patch fixes most of the style warnings found with checkpatch.pl in the
> hfa384x_usb.c file.
> Signed-off-by: Alessandro Ghedini <al3xbio@xxxxxxxxx>

Hi Alessandro.

http://driverdev.linuxdriverproject.org/pipermail/devel/2010-April/005105.html
http://driverdev.linuxdriverproject.org/pipermail/devel/2010-April/005106.html

When you get feedback about style changes in your patch submission,
you should see if the feedback applies to more than just a single
instance in your patch.

> @@ -2451,7 +2451,8 @@ int hfa384x_drvr_readpda(hfa384x_t *hw, void *buf, unsigned int len)
>  		currpage = HFA384x_ADDR_CMD_MKPAGE(pdaloc[i].cardaddr);
>  		curroffset = HFA384x_ADDR_CMD_MKOFF(pdaloc[i].cardaddr);
>  
> -		result = hfa384x_dormem_wait(hw, currpage, curroffset, buf, len);	/* units of bytes */
> +		result = hfa384x_dormem_wait(hw, currpage, curroffset, \
> +					buf, len); /* units of bytes */

These unnecessary line continuations are many.
Please remove them.
 
> @@ -2610,20 +2611,21 @@ int hfa384x_drvr_start(hfa384x_t *hw)
>  	result = result2 = hfa384x_cmd_initialize(hw);
>  	if (result1 != 0) {
>  		if (result2 != 0) {
> -			printk(KERN_ERR
> -			       "cmd_initialize() failed on two attempts, results %d and %d\n",
> +			printk(KERN_ERR "cmd_initialize() failed on two" \
> +					"attempts, results %d and %d\n",

here.


> -			pr_debug
> -			    ("but second attempt succeeded. All should be ok\n");
> +			pr_debug("but second attempt succeeded. " \
> +						"All should be ok\n");

here, etc.

@@ -2075,12 +2078,9 @@ int hfa384x_drvr_flashdl_write(hfa384x_t *hw, u32 daddr, void *buf, u32 len)
>  			    (j * HFA384x_USB_RWMEM_MAXLEN);
>  
>  			writepage = HFA384x_ADDR_CMD_MKPAGE(dlbufaddr +
> -							    (j *
> -							     HFA384x_USB_RWMEM_MAXLEN));
> -			writeoffset =
> -			    HFA384x_ADDR_CMD_MKOFF(dlbufaddr +
> -						   (j *
> -						    HFA384x_USB_RWMEM_MAXLEN));
> +					(j * HFA384x_USB_RWMEM_MAXLEN));
> +			writeoffset = HFA384x_ADDR_CMD_MKOFF(dlbufaddr +
> +					(j * HFA384x_USB_RWMEM_MAXLEN));
>  
>  			writelen = burnlen - (j * HFA384x_USB_RWMEM_MAXLEN);

Try not to be slavish in simply fixing the checkpatch
errors.  The goal isn't just to keep the silly tool quiet
but to make code readable and maintainable while making it
reasonably conforming to the predominant kernel coding
styles.

This code could be simplified and made much more readable
by using a temporary for (j * HFA384x_USB_RWMEM_MAXLEN)

Something like:

		for (j = 0; j < nwrites; j++) {
			int pos = j * HFA384x_USB_RWMEM_MAXLEN; 
			writebuf = buf + (i * hw->bufinfo.len) + pos;
			writepage = HFA384x_ADDR_CMD_MKPAGE(dlbufaddr + pos);
			writeoffset = HFA384x_ADDR_CMD_MKOFF(dlbufaddr + pos);
			writelen = burnlen - jmax;

> @@ -3583,8 +3586,10 @@ static void hfa384x_int_rxmonitor(wlandevice_t *wlandev,
>  	struct sk_buff *skb;
>  	hfa384x_t *hw = wlandev->priv;
>  
> -	/* Don't forget the status, time, and data_len fields are in host order */
> -	/* Figure out how big the frame is */
> +	/* Don't forget the status, time, and data_len fields are in
> +	 * host order
> +	 * Figure out how big the frame is
> +	 */

If you're going to convert something like this
perhaps a better style would be:

	/*
	 * Figure out how big the frame is
	 * The status, time and data_len fields are in host order
	 */

>  	fc = le16_to_cpu(rxdesc->frame_control);
>  	hdrlen = p80211_headerlen(fc);
>  	datalen = le16_to_cpu(rxdesc->data_len);
> @@ -3632,7 +3637,9 @@ static void hfa384x_int_rxmonitor(wlandevice_t *wlandev,
>  		caphdr->encoding = htonl(1);	/* cck */
>  	}
>  
> -	/* Copy the 802.11 header to the skb (ctl frames may be less than a full header) */
> +	/* Copy the 802.11 header to the skb (ctl frames may be less
> +	 * than a full header)

If you're breaking comments up like this, use some taste.
Don't necessarily fill all comments to the maximum line length.

	/*
	 * Copy the 80211.11 header to the skb
	 * (ctl frames may be less than a full header)
	 */

cheers, Joe

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux