Skip to contentSkip to conversation

staging: greybus: arche: wrap over-length lines

PATCH Superseded Mitchell Tasman 4 participants 8 messages Jul 9, 2017 – Jul 11, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: greybus: arche: wrap over-length lines

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

---
 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +19 -10
index eced2d2..c386c13 100644
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 		if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
 			if (time_before(jiffies,
 					arche_pdata->wake_detect_start +
-					msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-				arche_platform_set_wake_detect_state(arche_pdata,
-								     WD_STATE_IDLE);
+					msecs_to_jiffies(
+						WD_COLDBOOT_PULSE_WIDTH_MS))) {
+				arche_platform_set_wake_detect_state(
+								arche_pdata,
+								WD_STATE_IDLE);
 			} else {
-				/* Check we are not in middle of irq thread already */
+				/*
+				 * Check we are not in middle of irq thread
+				 * already
+				 */
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
-					arche_platform_set_wake_detect_state(arche_pdata,
-									     WD_STATE_COLDBOOT_TRIG);
+					arche_platform_set_wake_detect_state(
+							arche_pdata,
+							WD_STATE_COLDBOOT_TRIG);
 					spin_unlock_irqrestore(
 						&arche_pdata->wake_lock,
 						flags);
@@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 			 * beyond 30msec, then it is coldboot else fallback
 			 * to standby boot.
 			 */
-			arche_platform_set_wake_detect_state(arche_pdata,
-							     WD_STATE_BOOT_INIT);
+			arche_platform_set_wake_detect_state(
+							arche_pdata,
+							WD_STATE_BOOT_INIT);
 		}
 	}
 
@@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
 			arche_platform_resume);
 
 static const struct of_device_id arche_platform_of_match[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device*/
+	{ .compatible = "google,arche-platform", },
 	{ },
 };
 
 static const struct of_device_id arche_combined_id[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ .compatible = "usbffff,2", },
 	{ },
 };
Signature · 1 line
2.9.3
#1 lore raw

PATCH v2 1/1 staging: greybus: arche: wrap over-length lines

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

---
Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +19 -10
index eced2d2..eedd239 100644
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 		if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
 			if (time_before(jiffies,
 					arche_pdata->wake_detect_start +
-					msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-				arche_platform_set_wake_detect_state(arche_pdata,
-								     WD_STATE_IDLE);
+					msecs_to_jiffies(
+						WD_COLDBOOT_PULSE_WIDTH_MS))) {
+				arche_platform_set_wake_detect_state(
+								arche_pdata,
+								WD_STATE_IDLE);
 			} else {
-				/* Check we are not in middle of irq thread already */
+				/*
+				 * Check we are not in middle of irq thread
+				 * already
+				 */
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
-					arche_platform_set_wake_detect_state(arche_pdata,
-									     WD_STATE_COLDBOOT_TRIG);
+					arche_platform_set_wake_detect_state(
+							arche_pdata,
+							WD_STATE_COLDBOOT_TRIG);
 					spin_unlock_irqrestore(
 						&arche_pdata->wake_lock,
 						flags);
@@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 			 * beyond 30msec, then it is coldboot else fallback
 			 * to standby boot.
 			 */
-			arche_platform_set_wake_detect_state(arche_pdata,
-							     WD_STATE_BOOT_INIT);
+			arche_platform_set_wake_detect_state(
+							arche_pdata,
+							WD_STATE_BOOT_INIT);
 		}
 	}
 
@@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
 			arche_platform_resume);
 
 static const struct of_device_id arche_platform_of_match[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ },
 };
 
 static const struct of_device_id arche_combined_id[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ .compatible = "usbffff,2", },
 	{ },
 };
Signature · 1 line
2.9.3
#2 lore raw

Hi Mitchell,

Quoted 2 lines · Mitchell Tasman

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

We try to follow that most of the time, but the end result should be easily readable. If it isn't, then we just ignore the rule.

Quoted 12 lines

---
Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +0 -0
index eced2d2..eedd239 100644

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 177

Quoted 9 lines
drivers/staging/greybus/arche-platform.c +3 -3
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 		if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
 			if (time_before(jiffies,
 					arche_pdata->wake_detect_start +
-					msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-				arche_platform_set_wake_detect_state(arche_pdata,
-								     WD_STATE_IDLE);
+					msecs_to_jiffies(
+						WD_COLDBOOT_PULSE_WIDTH_MS))) {
+				arche_platform_set_wake_detect_state(

We don't break the lines like this in kernel to satisfy the 80 column width rule. Surely, some places would have similar code, but in general this isn't recommended. And that's why we never bothered about 80 column rule in this and below cases.

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 184

Quoted 8 lines
drivers/staging/greybus/arche-platform.c +6 -1
+								arche_pdata,
+								WD_STATE_IDLE);
 			} else {
-				/* Check we are not in middle of irq thread already */
+				/*
+				 * Check we are not in middle of irq thread
+				 * already
+				 */

Yes, such changes would be fine.

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 187

Quoted 5 lines
drivers/staging/greybus/arche-platform.c +1 -2
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
-					arche_platform_set_wake_detect_state(arche_pdata,
-									     WD_STATE_COLDBOOT_TRIG);
+					arche_platform_set_wake_detect_state(

Not this.

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 208

Quoted 12 lines
drivers/staging/greybus/arche-platform.c +3 -2
+							arche_pdata,
+							WD_STATE_COLDBOOT_TRIG);
 					spin_unlock_irqrestore(
 						&arche_pdata->wake_lock,
 						flags);
@@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 			 * beyond 30msec, then it is coldboot else fallback
 			 * to standby boot.
 			 */
-			arche_platform_set_wake_detect_state(arche_pdata,
-							     WD_STATE_BOOT_INIT);
+			arche_platform_set_wake_detect_state(

Not this.

drivers/staging/greybus/arche-platform.c static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops, · line 668

Quoted 12 lines
drivers/staging/greybus/arche-platform.c +4 -1
+							arche_pdata,
+							WD_STATE_BOOT_INIT);
 		}
 	}
 
@@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
 			arche_platform_resume);
 
 static const struct of_device_id arche_platform_of_match[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },

This would be fine though.

Quoted 10 lines
drivers/staging/greybus/arche-platform.c +2 -1
 	{ },
 };
 
 static const struct of_device_id arche_combined_id[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ .compatible = "usbffff,2", },
 	{ },
 };
Signature · 1 line
viresh
#3 lore raw
Quoted 27 lines · Viresh Kumar

Hi Mitchell,

Quoted 2 lines · Mitchell Tasman

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

We try to follow that most of the time, but the end result should be easily readable. If it isn't, then we just ignore the rule.

Quoted 18 lines

---
Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +0 -3
index eced2d2..eedd239 100644
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
              if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
                      if (time_before(jiffies,
                                      arche_pdata->wake_detect_start +
-                                     msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-                             arche_platform_set_wake_detect_state(arche_pdata,
-                                                                  WD_STATE_IDLE);
Quoted 8 lines

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 177

Quoted 3 lines
drivers/staging/greybus/arche-platform.c +3 -0
+                                     msecs_to_jiffies(
+                                             WD_COLDBOOT_PULSE_WIDTH_MS))) {
+                             arche_platform_set_wake_detect_state(

We don't break the lines like this in kernel to satisfy the 80 column width rule. Surely, some places would have similar code, but in general this isn't recommended. And that's why we never bothered about 80 column rule in this and below cases.

In cases like this, one could argue that you should start looking at the level of indentation, rather than the line length per se. After all, the documentation states that "if you need more than 3 levels of indentation, you're screwed anyway, and you should fix your program".

#4 lore raw
Quoted 37 lines · Frans Klaver
Quoted 27 lines · Viresh Kumar

Hi Mitchell,

Quoted 2 lines · Mitchell Tasman

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

We try to follow that most of the time, but the end result should be easily readable. If it isn't, then we just ignore the rule.

Quoted 18 lines

---
Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +0 -3
index eced2d2..eedd239 100644
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
              if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
                      if (time_before(jiffies,
                                      arche_pdata->wake_detect_start +
-                                     msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-                             arche_platform_set_wake_detect_state(arche_pdata,
-                                                                  WD_STATE_IDLE);
Quoted 8 lines

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 177

Quoted 3 lines
drivers/staging/greybus/arche-platform.c +3 -0
+                                     msecs_to_jiffies(
+                                             WD_COLDBOOT_PULSE_WIDTH_MS))) {
+                             arche_platform_set_wake_detect_state(

We don't break the lines like this in kernel to satisfy the 80 column width rule. Surely, some places would have similar code, but in general this isn't recommended. And that's why we never bothered about 80 column rule in this and below cases.

Quoted 4 lines

In cases like this, one could argue that you should start looking at the level of indentation, rather than the line length per se. After all, the documentation states that "if you need more than 3 levels of indentation, you're screwed anyway, and you should fix your program".

And I completely agree to that :)

Signature · 1 line
viresh
#5 lore raw
Quoted 38 lines · Viresh Kumar
Quoted 37 lines · Frans Klaver
Quoted 35 lines · Viresh Kumar

Hi Mitchell,

Quoted 2 lines · Mitchell Tasman

Adjust formatting of various statements to keep line length within the 80 column limit preferred by the Linux kernel coding style.

We try to follow that most of the time, but the end result should be easily readable. If it isn't, then we just ignore the rule.

Quoted 12 lines

---
Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-platform.c +0 -0
index eced2d2..eedd239 100644

drivers/staging/greybus/arche-platform.c static irqreturn_t arche_platform_wd_irq(int irq, void *devid) · line 177

Quoted 9 lines
drivers/staging/greybus/arche-platform.c +3 -3
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
              if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
                      if (time_before(jiffies,
                                      arche_pdata->wake_detect_start +
-                                     msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-                             arche_platform_set_wake_detect_state(arche_pdata,
-                                                                  WD_STATE_IDLE);
+                                     msecs_to_jiffies(
+                                             WD_COLDBOOT_PULSE_WIDTH_MS))) {
+                             arche_platform_set_wake_detect_state(

We don't break the lines like this in kernel to satisfy the 80 column width rule. Surely, some places would have similar code, but in general this isn't recommended. And that's why we never bothered about 80 column rule in this and below cases.

Quoted 6 lines
Quoted 4 lines

In cases like this, one could argue that you should start looking at the level of indentation, rather than the line length per se. After all, the documentation states that "if you need more than 3 levels of indentation, you're screwed anyway, and you should fix your program".

And I completely agree to that :)

Another thing to consider is to s/arche_platform_// as the identifier lengths are just way too long.

#6 lore raw

PATCH v3 1/1 staging: greybus: arche: wrap over-length lines

In reply to Viresh Kumar

Adjust formatting of several comments to keep line length within the 80 column limit preferred by the Linux kernel coding style.

---
Changes in v3: Narrow scope of patch per Viresh Kumar's feedback Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/staging/greybus/arche-platform.c +8 -3
index eced2d2..6259904 100644
@@ -176,7 +176,10 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 				arche_platform_set_wake_detect_state(arche_pdata,
 								     WD_STATE_IDLE);
 			} else {
-				/* Check we are not in middle of irq thread already */
+				/*
+				 * Check we are not in middle of irq thread
+				 * already
+				 */
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
 					arche_platform_set_wake_detect_state(arche_pdata,
@@ -657,12 +660,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
 			arche_platform_resume);
 
 static const struct of_device_id arche_platform_of_match[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ },
 };
 
 static const struct of_device_id arche_combined_id[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ .compatible = "usbffff,2", },
 	{ },
 };
Signature · 1 line
2.9.3
Acked-by on v3 1/1
#7 lore raw
Quoted 34 lines · Mitchell Tasman

Adjust formatting of several comments to keep line length within the 80 column limit preferred by the Linux kernel coding style.

---
Changes in v3: Narrow scope of patch per Viresh Kumar's feedback Changes in v2: Add back a missing space in a comment

 drivers/staging/greybus/arche-platform.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/staging/greybus/arche-platform.c +6 -2
index eced2d2..6259904 100644
@@ -176,7 +176,10 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 				arche_platform_set_wake_detect_state(arche_pdata,
 								     WD_STATE_IDLE);
 			} else {
-				/* Check we are not in middle of irq thread already */
+				/*
+				 * Check we are not in middle of irq thread
+				 * already
+				 */
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
 					arche_platform_set_wake_detect_state(arche_pdata,
@@ -657,12 +660,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
 			arche_platform_resume);
 
 static const struct of_device_id arche_platform_of_match[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },

drivers/staging/greybus/arche-platform.c static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops, · line 673

Quoted 10 lines
drivers/staging/greybus/arche-platform.c +2 -1
 	{ },
 };
 
 static const struct of_device_id arche_combined_id[] = {
-	{ .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
+	/* Use PID/VID of SVC device */
+	{ .compatible = "google,arche-platform", },
 	{ .compatible = "usbffff,2", },
 	{ },
 };
Signature · 1 line
viresh