Staging: et131x: fix coding style issues in et131x_initpci.c

PATCH In review Joe Eloff 3 participants 4 messages Jul 6, 2010 – Jul 7, 2010 Staging

lore

Share & tools

lore

Joe Eloff

author

2010-07-06 18:19 UTC #0 lore raw

PATCH 1/1 Staging: et131x: fix coding style issues in et131x_initpci.c

Quoted 1 line

From e23e19537c4d62bc76ae982859d3c3225a45d9c2 Mon Sep 17 00:00:00 2001

From: Joe Eloff <[email protected]>
Date: Tue, 6 Jul 2010 19:13:28 +0200
Subject: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c

This is a patch to the et131x_initpci.c file that fixes up all the line lengths over 80 by the checkpatch.pl tool.

Signed-off-by Joe Eloff <[email protected]>

---
 drivers/staging/et131x/et131x_initpci.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

drivers/staging/et131x/et131x_initpci.c +10 -5

index 47baab3..ea93f83 100644
@@ -205,7 +205,8 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 		if (pci_write_config_word(pdev, ET1310_PCI_REPLAY,
 					       Replay[max_payload])) {
 			dev_err(&pdev->dev,
-			  "Could not write PCI config space for Replay Timer\n");
+			  "Could not write PCI config space for Replay \
+			   Timer\n");
 			return -EIO;
 		}
 	}

@@ -246,7 +247,8 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 	for (i = 0; i < ETH_ALEN; i++) {
 		if (pci_read_config_byte(pdev, ET1310_PCI_MAC_ADDRESS + i,
 					adapter->PermanentAddress + i)) {
-			dev_err(&pdev->dev, "Could not read PCI config space for MAC address\n");
+			dev_err(&pdev->dev, "Could not read PCI config space \
+					     for MAC address\n");
 			return -EIO;
 		}
 	}

@@ -539,10 +541,12 @@ static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev,
 
 	struct et131x_adapter *etdev;
 
-	/* Setup the fundamental net_device and private adapter structure elements  */
+	/* Setup the fundamental net_device and private
+	   adapter structure elements  */
 	SET_NETDEV_DEV(netdev, &pdev->dev);
 
-	/* Allocate private adapter struct and copy in relevant information */
+	/* Allocate private adapter struct and copy
+	   in relevant information */
 	etdev = netdev_priv(netdev);
 	etdev->pdev = pci_dev_get(pdev);
 	etdev->netdev = netdev;

@@ -655,7 +659,8 @@ static int __devinit et131x_pci_setup(struct pci_dev *pdev,
 		result = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
 		if (result != 0) {
 			dev_err(&pdev->dev,
-				  "Unable to obtain 64 bit DMA for consistent allocations\n");
+				  "Unable to obtain 64 bit DMA for \
+				   consistent allocations\n");
 			goto err_release_res;
 		}
 	} else if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))) {

Signature · 1 line

1.6.3.3

Nick Bowler

2010-07-06 19:26 UTC #1 lore raw

Quoted 17 lines · Joe Eloff

Quoted 1 line
From e23e19537c4d62bc76ae982859d3c3225a45d9c2 Mon Sep 17 00:00:00 2001
From: Joe Eloff <[email protected]>
Date: Tue, 6 Jul 2010 19:13:28 +0200
Subject: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c
This is a patch to the et131x_initpci.c file that fixes up all the line lengths over 80 by the checkpatch.pl tool.
Signed-off-by Joe Eloff <[email protected]>
---
 drivers/staging/et131x/et131x_initpci.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)
drivers/staging/et131x/et131x_initpci.c +0 -0
index 47baab3..ea93f83 100644

drivers/staging/et131x/et131x_initpci.c static int et131x_pci_init(struct et131x_adapter *adapter, · line 209

Quoted 6 lines

drivers/staging/et131x/et131x_initpci.c +2 -1

@@ -205,7 +205,8 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 		if (pci_write_config_word(pdev, ET1310_PCI_REPLAY,
 					       Replay[max_payload])) {
 			dev_err(&pdev->dev,
-			  "Could not write PCI config space for Replay Timer\n");
+			  "Could not write PCI config space for Replay \
+			   Timer\n");

First off, this change is wrong: you've just changed the error message to contain a huge amount of white space between the words Replay and Timer.

Secondly, please don't split printk strings onto multiple lines, because it makes them impossible to grep for.

These same comments apply to the other printk hunks in this patch.

[...]

drivers/staging/et131x/et131x_initpci.c static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev, · line 545

Quoted 7 lines

drivers/staging/et131x/et131x_initpci.c +2 -1

@@ -539,10 +541,12 @@ static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev,
 
 	struct et131x_adapter *etdev;
 
-	/* Setup the fundamental net_device and private adapter structure elements  */
+	/* Setup the fundamental net_device and private
+	   adapter structure elements  */

This new comment is much less readable than before due to the linebreak, IMO. Since the private adapter stuff is addressed by the subsequent comment (bit rot?), we can simply remove that part of the sentence and it will be shorter:

Setup the fundamental net_device structure elements.

drivers/staging/et131x/et131x_initpci.c static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev, · line 549

Quoted 5 lines

drivers/staging/et131x/et131x_initpci.c +2 -1

 	SET_NETDEV_DEV(netdev, &pdev->dev);
 
-	/* Allocate private adapter struct and copy in relevant information */
+	/* Allocate private adapter struct and copy
+	   in relevant information */

This comment didn't exceed 80 columns to begin with.

Signature · 1 line

Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Alan Cox

2010-07-06 21:58 UTC #2 lore raw

In reply to Joe Eloff

Quoted 9 lines · On Tue, 06 Jul 2010 19:17:08 +0200 Joe Eloff <[email protected]> wrote:

Quoted 2 lines
From e23e19537c4d62bc76ae982859d3c3225a45d9c2 Mon Sep 17 00:00:00 2001
From: Joe Eloff <[email protected]>
Date: Tue, 6 Jul 2010 19:13:28 +0200
Subject: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c
This is a patch to the et131x_initpci.c file that fixes up all the line lengths over 80 by the checkpatch.pl tool.

That is one area where checkpatch can be misleading. If you split messages like that it becomes hard to grep for them.

Where it is useful is where you have

printk("blah blah bah blah [120 chars]", functioncall(foo))

and it would hide things like functioncall(foo)

No argument about the comment ones however.

Joe Eloff

author

2010-07-07 05:43 UTC #3 lore raw

Quoted 16 lines · Alan Cox

Quoted 9 lines · On Tue, 06 Jul 2010 19:17:08 +0200 Joe Eloff <[email protected]> wrote:
Quoted 2 lines
From e23e19537c4d62bc76ae982859d3c3225a45d9c2 Mon Sep 17 00:00:00 2001
From: Joe Eloff <[email protected]>
Date: Tue, 6 Jul 2010 19:13:28 +0200
Subject: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c
This is a patch to the et131x_initpci.c file that fixes up all the line lengths over 80 by the checkpatch.pl tool.
That is one area where checkpatch can be misleading. If you split messages like that it becomes hard to grep for them.

Quoted 7 lines

Where it is useful is where you have
printk("blah blah bah blah [120 chars]", functioncall(foo))
and it would hide things like functioncall(foo)
No argument about the comment ones however.

Noted and understand why you would want to grep and output when debugging.
Will make better judgement calls thanks to these messages.

Regards,

Joe