diff mbox

bcma: fix three coding style issues, more than 80 characters per line

Message ID 1419708268-14114-1-git-send-email-oscar.forner.martinez@gmail.com (mailing list archive)
State Accepted
Delegated to: Kalle Valo
Headers show

Commit Message

Oscar Forner Martinez Dec. 27, 2014, 7:24 p.m. UTC
Three lines with more than 80 characters per line have been split in several lines.

Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
---
 drivers/bcma/driver_chipcommon.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Rafał Miłecki Dec. 27, 2014, 8:19 p.m. UTC | #1
On 27 December 2014 at 20:24, Oscar Forner Martinez
<oscar.forner.martinez@gmail.com> wrote:
> Three lines with more than 80 characters per line have been split in several lines.

Thanks for the next version :)

1) You missed version number. Something like
git format-patch --subject-prefix="PATCH V3"
(you saying for the future).

2) I'm OK with your changes, but when checking the patch I get some errors like
ERROR: trailing whitespace
ERROR: DOS line endings
(I used ./scripts/checkpatch.pl, see attached checkpatch log).

I think that "trailing whitespace" errors are caused by DOS line
endings, because I don't see anything wrong about your code.

Is there something wrong with your emacs? git-send-email shouldn't
malform patches I believe.
ERROR: trailing whitespace
#62: FILE: drivers/bcma/driver_chipcommon.c:82:
+^I^I^I/* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP^M$

ERROR: trailing whitespace
#63: FILE: drivers/bcma/driver_chipcommon.c:83:
+^I^I^I * clock^M$

ERROR: DOS line endings
#64: FILE: drivers/bcma/driver_chipcommon.c:84:
+^I^I^I */^M$

ERROR: DOS line endings
#73: FILE: drivers/bcma/driver_chipcommon.c:102:
+^Iwdt.max_timer_ms =^M$

ERROR: DOS line endings
#74: FILE: drivers/bcma/driver_chipcommon.c:103:
+^I^Ibcma_chipco_watchdog_get_max_timer(cc) / cc->ticks_per_ms;^M$

ERROR: DOS line endings
#83: FILE: drivers/bcma/driver_chipcommon.c:341:
+^I^Ibcma_err(cc->core->bus, "serial not supported on this device ccrev: 0x%x\n",^M$

ERROR: DOS line endings
#84: FILE: drivers/bcma/driver_chipcommon.c:342:
+^I^I^I ccrev);^M$

total: 7 errors, 0 warnings, 28 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

bcma.patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Rafał Miłecki Dec. 27, 2014, 11:42 p.m. UTC | #2
On 27 December 2014 at 23:04, Oscar Forner Martinez
<oscar.forner.martinez@gmail.com> wrote:
> 2014-12-27 21:14 GMT+00:00 Oscar Forner Martinez
> <oscar.forner.martinez@gmail.com>:
>>
>> 2014-12-27 20:19 GMT+00:00 Rafa? Mi?ecki <zajec5@gmail.com>:
>>>
>>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>>> <oscar.forner.martinez@gmail.com> wrote:
>>> > Three lines with more than 80 characters per line have been split in
>>> > several lines.
>>>
>>> Thanks for the next version :)
>>>
>>> 1) You missed version number. Something like
>>> git format-patch --subject-prefix="PATCH V3"
>>> (you saying for the future).
>>
>>
>> Yes, sorry I forgot to put -v 3 in git format-patch.
>>
>>>
>>>
>>> 2) I'm OK with your changes, but when checking the patch I get some
>>> errors like
>>> ERROR: trailing whitespace
>>> ERROR: DOS line endings
>>> (I used ./scripts/checkpatch.pl, see attached checkpatch log).
>>>
>>> I think that "trailing whitespace" errors are caused by DOS line
>>> endings, because I don't see anything wrong about your code.
>>>
>>> Is there something wrong with your emacs? git-send-email shouldn't
>>> malform patches I believe.
>>
>>
>> When I run ./scripts/checkpatch.pl with my patch does not show any error
>> (please find attached the output). I will try to run it with the one from my
>> e-mail. My only configuration in the .emacs file is: (setq c-default-style
>> "linux").
>
>
> I ran the ./scripts/checkpatch.pl with the patch got it from
> http://marc.info/?l=linux-wireless&m=141970834908165 and it does not show
> any error. Could you please tell me how I have to check in the same way as
> you did?

False alarm, sorry. GMail changed sth again or my browser
change/update broke saving e-mails.
Rafał Miłecki Dec. 27, 2014, 11:44 p.m. UTC | #3
On 27 December 2014 at 20:24, Oscar Forner Martinez
<oscar.forner.martinez@gmail.com> wrote:
> Three lines with more than 80 characters per line have been split in several lines.
>
> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>

Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>

Kalle: will you pick this patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sedat Dilek Dec. 28, 2014, 5:50 a.m. UTC | #4
On Sun, Dec 28, 2014 at 12:44 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> On 27 December 2014 at 20:24, Oscar Forner Martinez
> <oscar.forner.martinez@gmail.com> wrote:
>> Three lines with more than 80 characters per line have been split in several lines.
>>
>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>
> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>

As for the comment-line changes... 80+ chars are allowed for better readability.
So, please don't do that.
[ Checkpatch should not warn on this especially for comments. ]

- Sedat -

> Kalle: will you pick this patch?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafał Miłecki Dec. 28, 2014, 8:53 a.m. UTC | #5
On 28 December 2014 at 06:50, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Sun, Dec 28, 2014 at 12:44 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>> <oscar.forner.martinez@gmail.com> wrote:
>>> Three lines with more than 80 characters per line have been split in several lines.
>>>
>>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>>
>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>
>
> As for the comment-line changes... 80+ chars are allowed for better readability.
> So, please don't do that.
> [ Checkpatch should not warn on this especially for comments. ]

We almost always split long comments into separated lines in the
kernel. What's different with this case?
Sedat Dilek Dec. 28, 2014, 9:12 a.m. UTC | #6
On Sun, Dec 28, 2014 at 9:53 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> On 28 December 2014 at 06:50, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> On Sun, Dec 28, 2014 at 12:44 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>>> <oscar.forner.martinez@gmail.com> wrote:
>>>> Three lines with more than 80 characters per line have been split in several lines.
>>>>
>>>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>>>
>>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>>
>>
>> As for the comment-line changes... 80+ chars are allowed for better readability.
>> So, please don't do that.
>> [ Checkpatch should not warn on this especially for comments. ]
>
> We almost always split long comments into separated lines in the
> kernel. What's different with this case?
>

1st it is not mandatory.

2nd it is more readable in one line.

-                       /* 4706 CC and PMU watchdogs are clocked at
1/4 of ALP clock */
+                       /* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP
+                        * clock
+                        */

I agree with you when the comment would be longer.

- Sedat -
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafał Miłecki Dec. 28, 2014, 2:02 p.m. UTC | #7
On 28 December 2014 at 10:12, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Sun, Dec 28, 2014 at 9:53 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> On 28 December 2014 at 06:50, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>> On Sun, Dec 28, 2014 at 12:44 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>>>> <oscar.forner.martinez@gmail.com> wrote:
>>>>> Three lines with more than 80 characters per line have been split in several lines.
>>>>>
>>>>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>>>>
>>>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>>>
>>>
>>> As for the comment-line changes... 80+ chars are allowed for better readability.
>>> So, please don't do that.
>>> [ Checkpatch should not warn on this especially for comments. ]
>>
>> We almost always split long comments into separated lines in the
>> kernel. What's different with this case?
>>
>
> 1st it is not mandatory.
>
> 2nd it is more readable in one line.
>
> -                       /* 4706 CC and PMU watchdogs are clocked at
> 1/4 of ALP clock */
> +                       /* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP
> +                        * clock
> +                        */
>
> I agree with you when the comment would be longer.

So I guess there is some rule like
"Don't use 2 lines if there is 80 chars + one word"
? Where can I find such rules?
Arend van Spriel Dec. 28, 2014, 3:01 p.m. UTC | #8
On 12/28/14 15:02, Rafa? Mi?ecki wrote:
> On 28 December 2014 at 10:12, Sedat Dilek<sedat.dilek@gmail.com>  wrote:
>> On Sun, Dec 28, 2014 at 9:53 AM, Rafa? Mi?ecki<zajec5@gmail.com>  wrote:
>>> On 28 December 2014 at 06:50, Sedat Dilek<sedat.dilek@gmail.com>  wrote:
>>>> On Sun, Dec 28, 2014 at 12:44 AM, Rafa? Mi?ecki<zajec5@gmail.com>  wrote:
>>>>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>>>>> <oscar.forner.martinez@gmail.com>  wrote:
>>>>>> Three lines with more than 80 characters per line have been split in several lines.
>>>>>>
>>>>>> Signed-off-by: Oscar Forner Martinez<oscar.forner.martinez@gmail.com>
>>>>>
>>>>> Acked-by: Rafa? Mi?ecki<zajec5@gmail.com>
>>>>>
>>>>
>>>> As for the comment-line changes... 80+ chars are allowed for better readability.
>>>> So, please don't do that.
>>>> [ Checkpatch should not warn on this especially for comments. ]
>>>
>>> We almost always split long comments into separated lines in the
>>> kernel. What's different with this case?
>>>
>>
>> 1st it is not mandatory.
>>
>> 2nd it is more readable in one line.
>>
>> -                       /* 4706 CC and PMU watchdogs are clocked at
>> 1/4 of ALP clock */
>> +                       /* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP
>> +                        * clock
>> +                        */
>>
>> I agree with you when the comment would be longer.
>
> So I guess there is some rule like
> "Don't use 2 lines if there is 80 chars + one word"
> ? Where can I find such rules?

Personally, I don't like taking this sliding slope. Just stick to the 80 
characters rule and add a few words in the comment, eg:

			/* The BCM4706 ChipCommon and PMU watchdogs are
			 * clocked at one quarter of the ALP clock.
			 */

Regards,
Arend

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo Dec. 29, 2014, 7:16 a.m. UTC | #9
Rafa? Mi?ecki <zajec5@gmail.com> writes:

> On 27 December 2014 at 20:24, Oscar Forner Martinez
> <oscar.forner.martinez@gmail.com> wrote:
>> Three lines with more than 80 characters per line have been split in several lines.
>>
>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>
> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>
> Kalle: will you pick this patch?

Most likely yes, but let's wait first for John's opinion about how to
handle bcma patches.
Oscar Forner Martinez Jan. 7, 2015, 11:18 a.m. UTC | #10
That e-mail is just to remind that this patch stills waiting.

On 29 Dec 2014, at 07:16, Kalle Valo <kvalo@codeaurora.org> wrote:

> Rafa? Mi?ecki <zajec5@gmail.com> writes:
> 
>> On 27 December 2014 at 20:24, Oscar Forner Martinez
>> <oscar.forner.martinez@gmail.com> wrote:
>>> Three lines with more than 80 characters per line have been split in several lines.
>>> 
>>> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>
>> 
>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>> 
>> Kalle: will you pick this patch?
> 
> Most likely yes, but let's wait first for John's opinion about how to
> handle bcma patches.
> 
> -- 
> Kalle Valo

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo Jan. 7, 2015, 1:57 p.m. UTC | #11
Òscar Forner Martínez <oscar.forner.martinez@gmail.com> writes:

> That e-mail is just to remind that this patch stills waiting.

Just like the other 150 patches as of yesterday. All I can say please wait
patiently and follow the status in patchwork. Your patch isn't urgent in
any way.

> On 29 Dec 2014, at 07:16, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafa? Mi?ecki <zajec5@gmail.com> writes:
>> 
>>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>> 
>>> Kalle: will you pick this patch?
>> 
>> Most likely yes, but let's wait first for John's opinion about how to
>> handle bcma patches.

I talked with John about this and I'll start taking bcma patches from
now on.
Oscar Forner Martinez Jan. 7, 2015, 2:01 p.m. UTC | #12
I know my patch is not urgent at all. My e-mail was just to remind that stills there. I do not know how much time I have to wait until I should remind a patch.

On 7 Jan 2015, at 13:57, Kalle Valo <kvalo@codeaurora.org> wrote:

> Òscar Forner Martínez <oscar.forner.martinez@gmail.com> writes:
> 
>> That e-mail is just to remind that this patch stills waiting.
> 
> Just like the other 150 patches as of yesterday. All I can say please wait
> patiently and follow the status in patchwork. Your patch isn't urgent in
> any way.
> 
>> On 29 Dec 2014, at 07:16, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Rafa? Mi?ecki <zajec5@gmail.com> writes:
>>> 
>>>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>>> 
>>>> Kalle: will you pick this patch?
>>> 
>>> Most likely yes, but let's wait first for John's opinion about how to
>>> handle bcma patches.
> 
> I talked with John about this and I'll start taking bcma patches from
> now on.
> 
> -- 
> Kalle Valo

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo Jan. 15, 2015, 12:42 p.m. UTC | #13
Oscar Forner Martinez <oscar.forner.martinez@gmail.com> writes:

> Three lines with more than 80 characters per line have been split in
> several lines.
>
> Signed-off-by: Oscar Forner Martinez <oscar.forner.martinez@gmail.com>

Thanks, applied to wireless-drivers-next.git.

Hopefully I took the correct version this time :)
diff mbox

Patch

diff --git a/drivers/bcma/driver_chipcommon.c b/drivers/bcma/driver_chipcommon.c
index 19f6796..84d4a95 100644
--- a/drivers/bcma/driver_chipcommon.c
+++ b/drivers/bcma/driver_chipcommon.c
@@ -79,7 +79,9 @@  static int bcma_chipco_watchdog_ticks_per_ms(struct bcma_drv_cc *cc)
 
 	if (cc->capabilities & BCMA_CC_CAP_PMU) {
 		if (bus->chipinfo.id == BCMA_CHIP_ID_BCM4706)
-			/* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP clock */
+			/* 4706 CC and PMU watchdogs are clocked at 1/4 of ALP
+			 * clock
+			 */
 			return bcma_chipco_get_alp_clock(cc) / 4000;
 		else
 			/* based on 32KHz ILP clock */
@@ -97,7 +99,8 @@  int bcma_chipco_watchdog_register(struct bcma_drv_cc *cc)
 	wdt.driver_data = cc;
 	wdt.timer_set = bcma_chipco_watchdog_timer_set_wdt;
 	wdt.timer_set_ms = bcma_chipco_watchdog_timer_set_ms_wdt;
-	wdt.max_timer_ms = bcma_chipco_watchdog_get_max_timer(cc) / cc->ticks_per_ms;
+	wdt.max_timer_ms =
+		bcma_chipco_watchdog_get_max_timer(cc) / cc->ticks_per_ms;
 
 	pdev = platform_device_register_data(NULL, "bcm47xx-wdt",
 					     cc->core->bus->num, &wdt,
@@ -335,7 +338,8 @@  void bcma_chipco_serial_init(struct bcma_drv_cc *cc)
 				       | BCMA_CC_CORECTL_UARTCLKEN);
 		}
 	} else {
-		bcma_err(cc->core->bus, "serial not supported on this device ccrev: 0x%x\n", ccrev);
+		bcma_err(cc->core->bus, "serial not supported on this device ccrev: 0x%x\n",
+			 ccrev);
 		return;
 	}