[PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[Daeseok Youn]

clean up checkpatch.pl warnings:
 WARNING: Line length over 80 characters

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
 drivers/staging/cxt1e1/hwprobe.c |   45 ++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c
index 85040bb..d6ccbd9 100644
--- a/drivers/staging/cxt1e1/hwprobe.c
+++ b/drivers/staging/cxt1e1/hwprobe.c
@@ -37,7 +37,8 @@ extern int  drvr_state;
 
 /* forward references */
 void        c4_stopwd(ci_t *);
-struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, unsigned long, int, int);
+struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long,
+				      unsigned long, int, int);
 
 
 struct s_hdw_info hdw_info[MAX_BOARDS];
@@ -104,24 +105,31 @@ hdw_sn_get(hdw_info_t *hi, int brdno)
 	addr = (long) hi->addr_mapped[1] + EEPROM_OFFSET;
 
 	/* read EEPROM with largest known format size... */
-	pmc_eeprom_read_buffer(addr, 0, (char *)hi->mfg_info.data, sizeof(FLD_TYPE2));
+	pmc_eeprom_read_buffer(addr, 0, (char *)hi->mfg_info.data,
+			       sizeof(FLD_TYPE2));
 
 #if 0
 	{
 		unsigned char *ucp = (unsigned char *) &hi->mfg_info.data;
 
 		pr_info("eeprom[00]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
+			*(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3),
+			*(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
 		pr_info("eeprom[08]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
+			*(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11),
+			*(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
 		pr_info("eeprom[16]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
+			*(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19),
+			*(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
 		pr_info("eeprom[24]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
+			*(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27),
+			*(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
 		pr_info("eeprom[32]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
+			*(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35),
+			*(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
 		pr_info("eeprom[40]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
-				*(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
+			*(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43),
+			*(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
 	}
 #endif
 #if 0
@@ -230,10 +238,11 @@ c4_hdw_init(struct pci_dev *pdev, int found)
 		return 0;
 	}
 
-	if (pdev->bus)                  /* obtain bus number */
+	/* obtain bus number */
+	if (pdev->bus)
 		busno = pdev->bus->number;
 	else
-		busno = 0;                  /* default for system PCI inconsistency */
+		busno = 0; /* default for system PCI inconsistency */
 	slot = pdev->devfn & ~0x07;
 
 	/*
@@ -246,8 +255,8 @@ c4_hdw_init(struct pci_dev *pdev, int found)
 	for (i = 0, hi = hdw_info; i < MAX_BOARDS; i++, hi++)
 	{
 		/*
-		 * match with board's first found interface, otherwise this is first
-		 * found
+		 * match with board's first found interface, otherwise this is
+		 * fisrt found
 		 */
 		if ((hi->pci_slot == 0xff) ||   /* new board */
 		    ((hi->pci_slot == slot) && (hi->bus == pdev->bus)))
@@ -256,13 +265,14 @@ c4_hdw_init(struct pci_dev *pdev, int found)
 	if (i == MAX_BOARDS)            /* no match in above loop means MAX
 					 * exceeded */
 	{
-		pr_warning("exceeded number of allowed devices (>%d)?\n", MAX_BOARDS);
+		pr_warning("exceeded number of allowed devices (>%d)?\n",
+			   MAX_BOARDS);
 		return 0;
 	}
 	if (pdev->bus)
 		hi->pci_busno = pdev->bus->number;
 	else
-		hi->pci_busno = 0;          /* default for system PCI inconsistency */
+		hi->pci_busno = 0; /* default for system PCI inconsistency */
 	hi->pci_slot = slot;
 	pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &hi->pci_pin[fun]);
 	pci_read_config_byte(pdev, PCI_REVISION_ID, &hi->revid[fun]);
@@ -345,7 +355,8 @@ c4hw_attach_all(void)
 			}
 #ifdef SBE_MAP_DEBUG
 			pr_warning("%s: io remapped from phys %x to virt %x\n",
-				   hi->devname, (u_int32_t) hi->addr[j], (u_int32_t) hi->addr_mapped[j]);
+				   hi->devname, (u_int32_t) hi->addr[j],
+				   (u_int32_t) hi->addr_mapped[j]);
 #endif
 		}
 	}
@@ -380,9 +391,9 @@ c4hw_attach_all(void)
 #if 0
 			cleanup_devs();
 #endif
-			return error_flag;      /* error_flag set w/in add_dev() */
+			return error_flag; /* error_flag set w/in add_dev() */
 		}
-		show_two(hi, i);           /* displays found information */
+		show_two(hi, i); /* displays found information */
 	}
 	return 0;
 }
-- 
1.7.9.5

---

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[Krzysztof Hałasa]

Daeseok Youn <daeseok.youn@gmail.com> writes:

> clean up checkpatch.pl warnings:
>  WARNING: Line length over 80 characters

This warning should be long gone IMHO. It does more harm than good.

>  		unsigned char *ucp = (unsigned char *) &hi->mfg_info.data;
>  
>  		pr_info("eeprom[00]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
> +			*(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3),
> +			*(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
>  		pr_info("eeprom[08]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
> +			*(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11),
> +			*(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
>  		pr_info("eeprom[16]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
> +			*(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19),
> +			*(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
>  		pr_info("eeprom[24]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
> +			*(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27),
> +			*(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
>  		pr_info("eeprom[32]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
> +			*(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35),
> +			*(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
>  		pr_info("eeprom[40]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
> -				*(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
> +			*(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43),
> +			*(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
>  	}

This one is probably an improvement, though... why not use some existing
*hex_dump*() instead?

> -		 * match with board's first found interface, otherwise this is first
> -		 * found
> +		 * match with board's first found interface, otherwise this is
> +		 * fisrt found
                   ^^^^^
I wonder what's your exact procedure for doing such changes :-)

>  #ifdef SBE_MAP_DEBUG
>  			pr_warning("%s: io remapped from phys %x to virt %x\n",
> -				   hi->devname, (u_int32_t) hi->addr[j], (u_int32_t) hi->addr_mapped[j]);
> +				   hi->devname, (u_int32_t) hi->addr[j],
> +				   (u_int32_t) hi->addr_mapped[j]);

This one is fine, too. The others, I wouldn't touch them.
-- 
Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[DaeSeok Youn]

2014-03-03 15:46 GMT+09:00 Krzysztof Hałasa <khalasa@piap.pl>:
> Daeseok Youn <daeseok.youn@gmail.com> writes:
>
>> clean up checkpatch.pl warnings:
>>  WARNING: Line length over 80 characters
>
> This warning should be long gone IMHO. It does more harm than good.
>
>>               unsigned char *ucp = (unsigned char *) &hi->mfg_info.data;
>>
>>               pr_info("eeprom[00]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
>> +                     *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3),
>> +                     *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7));
>>               pr_info("eeprom[08]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
>> +                     *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11),
>> +                     *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15));
>>               pr_info("eeprom[16]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
>> +                     *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19),
>> +                     *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23));
>>               pr_info("eeprom[24]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
>> +                     *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27),
>> +                     *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31));
>>               pr_info("eeprom[32]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
>> +                     *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35),
>> +                     *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39));
>>               pr_info("eeprom[40]:  %02x %02x %02x %02x  %02x %02x %02x %02x\n",
>> -                             *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
>> +                     *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43),
>> +                     *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47));
>>       }
>
> This one is probably an improvement, though... why not use some existing
> *hex_dump*() instead?
OK. but this patch is only for fix line length over 80 characters,
right? If it can be improved as your comment,
it must be in another patch.

>
>> -              * match with board's first found interface, otherwise this is first
>> -              * found
>> +              * match with board's first found interface, otherwise this is
>> +              * fisrt found
>                    ^^^^^
> I wonder what's your exact procedure for doing such changes :-)
"first" word is over 80 line character after applying my previous patch(2/5)
(previous patch is already merged to staging-next tree :-))
>
>>  #ifdef SBE_MAP_DEBUG
>>                       pr_warning("%s: io remapped from phys %x to virt %x\n",
>> -                                hi->devname, (u_int32_t) hi->addr[j], (u_int32_t) hi->addr_mapped[j]);
>> +                                hi->devname, (u_int32_t) hi->addr[j],
>> +                                (u_int32_t) hi->addr_mapped[j]);
>
> This one is fine, too. The others, I wouldn't touch them.
this line, same reason above one.
> --
> Krzysztof Halasa
>
> Research Institute for Automation and Measurements PIAP
> Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[Joe Perches]

On Mon, 2014-03-03 at 16:01 +0900, DaeSeok Youn wrote:
> 2014-03-03 15:46 GMT+09:00 Krzysztof Hałasa <khalasa@piap.pl>:
> > why not use some existing *hex_dump*() instead?
> OK. but this patch is only for fix line length over 80 characters,
> right? If it can be improved as your comment,
> it must be in another patch.

Not true.  Always strive to improve the code in
preference to shutting up checkpatch warnings.

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[DaeSeok Youn]

I think line break is a solution about long line warning. And
replacing with hex_dump() is better than managing with another patch.
So I said this improvement is separated with another patch.

Just my opinion.
If I'm wrong, I try to change as Krzysztof's comment in same patch.

Thanks.
Daeseok Youn.

2014-03-03 16:10 GMT+09:00 Joe Perches <joe@perches.com>:
> On Mon, 2014-03-03 at 16:01 +0900, DaeSeok Youn wrote:
>> 2014-03-03 15:46 GMT+09:00 Krzysztof Hałasa <khalasa@piap.pl>:
>> > why not use some existing *hex_dump*() instead?
>> OK. but this patch is only for fix line length over 80 characters,
>> right? If it can be improved as your comment,
>> it must be in another patch.
>
> Not true.  Always strive to improve the code in
> preference to shutting up checkpatch warnings.
>
>

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[DaeSeok Youn]

Hi, Joe and Krzysztof

I think that code which can be improved to hex_dump() may be used for debugging.
So that improvement will be sent with another patch.

I think it would be fine. right?

Thanks.
Daeseok Youn.



2014-03-03 16:33 GMT+09:00 DaeSeok Youn <daeseok.youn@gmail.com>:
> I think line break is a solution about long line warning. And
> replacing with hex_dump() is better than managing with another patch.
> So I said this improvement is separated with another patch.
>
> Just my opinion.
> If I'm wrong, I try to change as Krzysztof's comment in same patch.
>
> Thanks.
> Daeseok Youn.
>
> 2014-03-03 16:10 GMT+09:00 Joe Perches <joe@perches.com>:
>> On Mon, 2014-03-03 at 16:01 +0900, DaeSeok Youn wrote:
>>> 2014-03-03 15:46 GMT+09:00 Krzysztof Hałasa <khalasa@piap.pl>:
>>> > why not use some existing *hex_dump*() instead?
>>> OK. but this patch is only for fix line length over 80 characters,
>>> right? If it can be improved as your comment,
>>> it must be in another patch.
>>
>> Not true.  Always strive to improve the code in
>> preference to shutting up checkpatch warnings.
>>
>>

Re: [PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c

[Krzysztof Hałasa]

DaeSeok Youn <daeseok.youn@gmail.com> writes:

>>> -              * match with board's first found interface, otherwise this is first
>>> -              * found
>>> +              * match with board's first found interface, otherwise this is
>>> +              * fisrt found
>>                    ^^^^^
>> I wonder what's your exact procedure for doing such changes :-)
> "first" word is over 80 line character after applying my previous patch(2/5)
> (previous patch is already merged to staging-next tree :-))

Sure, I meant I'd just break the line without re-typing the first/fisrt
word manually... Never mind.

Anyway, I'd rather aim for improving overall code quality, not just
silence 80-char warning. I'm not a big fan of this warning.
-- 
Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland