2010-07-06 18:19:12

by Joe Eloff

[permalink] [raw]
Subject: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c

>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(-)

diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index 47baab3..ea93f83 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -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))) {
--
1.6.3.3



2010-07-06 19:26:27

by Nick Bowler

[permalink] [raw]
Subject: Re: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c

On 19:17 Tue 06 Jul , Joe Eloff wrote:
> >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(-)
>
> diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
> index 47baab3..ea93f83 100644
> --- a/drivers/staging/et131x/et131x_initpci.c
> +++ b/drivers/staging/et131x/et131x_initpci.c
> @@ -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.

[...]
> @@ -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.

> 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.

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

2010-07-06 21:58:12

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c

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

> >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.

2010-07-07 05:43:36

by Joe Eloff

[permalink] [raw]
Subject: Re: [PATCH] Staging: et131x: fix coding style issues in et131x_initpci.c

On Tue, 2010-07-06 at 22:29 +0100, Alan Cox wrote:
> On Tue, 06 Jul 2010 19:17:08 +0200
> Joe Eloff <[email protected]> wrote:
>
> > >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.
Noted and understand why you would want to grep and output when
debugging.
Will make better judgement calls thanks to these messages.

Regards,

Joe