linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Staging: greybus: Match alignment with open parenthesis.
@ 2017-07-23  8:39 Shreeya Patel
  2017-07-23  8:47 ` Johan Hovold
  2017-07-28  4:48 ` Greg KH
  0 siblings, 2 replies; 8+ messages in thread
From: Shreeya Patel @ 2017-07-23  8:39 UTC (permalink / raw)
  To: johan, elder, gregkh, greybus-dev, devel, linux-kernel; +Cc: Shreeya Patel

Alignment should match with open parenthesis.
This fixes the coding style issue.

Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
---
 drivers/staging/greybus/tools/loopback_test.c | 35 ++++++++++++---------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c
index 32a4369..a29e091a 100644
--- a/drivers/staging/greybus/tools/loopback_test.c
+++ b/drivers/staging/greybus/tools/loopback_test.c
@@ -420,10 +420,10 @@ void log_csv_error(int len, int err)
 }
 
 int format_output(struct loopback_test *t,
-			struct loopback_results *r,
-			const char *dev_name,
-			char *buf, int buf_len,
-			struct tm *tm)
+		  struct loopback_results *r,
+		  const char *dev_name,
+		  char *buf, int buf_len,
+		  struct tm *tm)
 {
 	int len = 0;
 
@@ -535,7 +535,7 @@ static int log_results(struct loopback_test *t)
 	*/
 	if (t->file_output && !t->porcelain) {
 		snprintf(file_name, sizeof(file_name), "%s_%d_%d.csv",
-			t->test_name, t->size, t->iteration_max);
+			 t->test_name, t->size, t->iteration_max);
 
 		fd = open(file_name, O_WRONLY | O_CREAT | O_APPEND, 0644);
 		if (fd < 0) {
@@ -549,8 +549,8 @@ static int log_results(struct loopback_test *t)
 			continue;
 
 		len = format_output(t, &t->devices[i].results,
-					t->devices[i].name,
-					data, sizeof(data), &tm);
+				    t->devices[i].name,
+				    data, sizeof(data), &tm);
 		if (t->file_output && !t->porcelain) {
 			ret = write(fd, data, len);
 			if (ret == -1)
@@ -562,7 +562,7 @@ static int log_results(struct loopback_test *t)
 
 	if (t->aggregate_output) {
 		len = format_output(t, &t->aggregate_results, "aggregate",
-					data, sizeof(data), &tm);
+				    data, sizeof(data), &tm);
 		if (t->file_output && !t->porcelain) {
 			ret = write(fd, data, len);
 			if (ret == -1)
@@ -623,14 +623,13 @@ int find_loopback_devices(struct loopback_test *t)
 		snprintf(d->name, MAX_STR_LEN, "gb_loopback%u", dev_id);
 
 		snprintf(d->sysfs_entry, MAX_SYSFS_PATH, "%s%s/",
-			t->sysfs_prefix, d->name);
+			 t->sysfs_prefix, d->name);
 
 		snprintf(d->debugfs_entry, MAX_SYSFS_PATH, "%sraw_latency_%s",
-			t->debugfs_prefix, d->name);
+			 t->debugfs_prefix, d->name);
 
 		if (t->debug)
-			printf("add %s %s\n", d->sysfs_entry,
-				d->debugfs_entry);
+			printf("add %s %s\n", d->sysfs_entry, d->debugfs_entry);
 	}
 
 	ret = 0;
@@ -802,16 +801,14 @@ static void prepare_devices(struct loopback_test *t)
 				t->iteration_max);
 
 		if (t->use_async) {
+			write_sysfs_val(t->devices[i].sysfs_entry, "async", 1);
 			write_sysfs_val(t->devices[i].sysfs_entry,
-				"async", 1);
+					"timeout", t->async_timeout);
 			write_sysfs_val(t->devices[i].sysfs_entry,
-				"timeout", t->async_timeout);
-			write_sysfs_val(t->devices[i].sysfs_entry,
-				"outstanding_operations_max",
-				t->async_outstanding_operations);
+					"outstanding_operations_max",
+					t->async_outstanding_operations);
 		} else
-			write_sysfs_val(t->devices[i].sysfs_entry,
-				"async", 0);
+			write_sysfs_val(t->devices[i].sysfs_entry, "async", 0);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-23  8:39 [PATCH] Staging: greybus: Match alignment with open parenthesis Shreeya Patel
@ 2017-07-23  8:47 ` Johan Hovold
  2017-07-23 12:10   ` Dan Carpenter
  2017-07-28  4:48 ` Greg KH
  1 sibling, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2017-07-23  8:47 UTC (permalink / raw)
  To: Shreeya Patel; +Cc: johan, elder, gregkh, greybus-dev, devel, linux-kernel

On Sun, Jul 23, 2017 at 02:09:57PM +0530, Shreeya Patel wrote:
> Alignment should match with open parenthesis.
> This fixes the coding style issue.

No, it need not. This is a choice left up to the author, even if
checkpatch happens to complain about it with the --strict option.

Thanks,
Johan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-23  8:47 ` Johan Hovold
@ 2017-07-23 12:10   ` Dan Carpenter
  2017-07-23 12:54     ` Johan Hovold
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2017-07-23 12:10 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Shreeya Patel, devel, gregkh, linux-kernel, greybus-dev

On Sun, Jul 23, 2017 at 10:47:43AM +0200, Johan Hovold wrote:
> On Sun, Jul 23, 2017 at 02:09:57PM +0530, Shreeya Patel wrote:
> > Alignment should match with open parenthesis.
> > This fixes the coding style issue.
> 
> No, it need not. This is a choice left up to the author, even if
> checkpatch happens to complain about it with the --strict option.
> 

This is staging code.  Don't try to fight checkpatch.pl in staging...
You're just making work for everyone else (me) who already just reviewed
this patch and I don't want to review it a few more times when the next
person sends the exact same patch.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-23 12:10   ` Dan Carpenter
@ 2017-07-23 12:54     ` Johan Hovold
  2017-07-24  7:08       ` Dan Carpenter
  0 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2017-07-23 12:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Johan Hovold, Shreeya Patel, devel, gregkh, linux-kernel, greybus-dev

On Sun, Jul 23, 2017 at 03:10:53PM +0300, Dan Carpenter wrote:
> On Sun, Jul 23, 2017 at 10:47:43AM +0200, Johan Hovold wrote:
> > On Sun, Jul 23, 2017 at 02:09:57PM +0530, Shreeya Patel wrote:
> > > Alignment should match with open parenthesis.
> > > This fixes the coding style issue.
> > 
> > No, it need not. This is a choice left up to the author, even if
> > checkpatch happens to complain about it with the --strict option.
> > 
> 
> This is staging code.  Don't try to fight checkpatch.pl in staging...
> You're just making work for everyone else (me) who already just reviewed
> this patch and I don't want to review it a few more times when the next
> person sends the exact same patch.

You can just ignore any greybus patches if you prefer.

I've considered patching checkpatch to drop --strict for at least this
subsystem, but not sure it's worth it.

Johan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-23 12:54     ` Johan Hovold
@ 2017-07-24  7:08       ` Dan Carpenter
  2017-07-28  4:49         ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2017-07-24  7:08 UTC (permalink / raw)
  To: Johan Hovold; +Cc: devel, gregkh, linux-kernel, greybus-dev, Shreeya Patel

I don't understand why greybus has to be special instead of the same as
everything else.  Who cares about this stuff really?  Just do whatever
is easiest and most common.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-23  8:39 [PATCH] Staging: greybus: Match alignment with open parenthesis Shreeya Patel
  2017-07-23  8:47 ` Johan Hovold
@ 2017-07-28  4:48 ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: Greg KH @ 2017-07-28  4:48 UTC (permalink / raw)
  To: Shreeya Patel; +Cc: johan, elder, greybus-dev, devel, linux-kernel

On Sun, Jul 23, 2017 at 02:09:57PM +0530, Shreeya Patel wrote:
> Alignment should match with open parenthesis.
> This fixes the coding style issue.
> 
> Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> ---
>  drivers/staging/greybus/tools/loopback_test.c | 35 ++++++++++++---------------
>  1 file changed, 16 insertions(+), 19 deletions(-)

Does not apply cleanly to my tree, can you please rebase it and resend
if you want to have this applied?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-24  7:08       ` Dan Carpenter
@ 2017-07-28  4:49         ` Greg KH
  2017-07-28  9:01           ` Johan Hovold
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2017-07-28  4:49 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Johan Hovold, devel, linux-kernel, Shreeya Patel, greybus-dev

On Mon, Jul 24, 2017 at 10:08:37AM +0300, Dan Carpenter wrote:
> I don't understand why greybus has to be special instead of the same as
> everything else.  Who cares about this stuff really?  Just do whatever
> is easiest and most common.

It's not special, and --strict should apply here as long as it is in
staging.  So I'll apply these types of things to prevent people from
feeling like they wasted their time fixing something up and getting it
rejected for no good reason.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Staging: greybus: Match alignment with open parenthesis.
  2017-07-28  4:49         ` Greg KH
@ 2017-07-28  9:01           ` Johan Hovold
  0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2017-07-28  9:01 UTC (permalink / raw)
  To: Greg KH
  Cc: Dan Carpenter, Johan Hovold, devel, linux-kernel, Shreeya Patel,
	greybus-dev

On Thu, Jul 27, 2017 at 09:49:32PM -0700, Greg KH wrote:
> On Mon, Jul 24, 2017 at 10:08:37AM +0300, Dan Carpenter wrote:
> > I don't understand why greybus has to be special instead of the same as
> > everything else.  Who cares about this stuff really?  Just do whatever
> > is easiest and most common.
> 
> It's not special, and --strict should apply here as long as it is in
> staging.  So I'll apply these types of things to prevent people from
> feeling like they wasted their time fixing something up and getting it
> rejected for no good reason.

Your call, but I do think that this is taking the checkpatch exercise
too far.

What we're talking about here are neither checkpatch ERRORs nor
WARNINGs, but that third category of CHECK notifications that are
output only when the switch --strict, or it's more aptly named alias
--subjective, is specified.

Just like we don't blindly apply patches addressing every ERROR or
WARNING, I think we should not accept every patch addressing these
subjective CHECK notifications.

These flags are useful when people are submitting new code; and most of
the CHECKs enabled by --subjective do seem reasonable, at least in my
(subjective) opinion.

I personally think that neither OPEN_PARENTHESIS nor LINE_SPACING do
or should apply everywhere however, and that the final call should be
left up to the author and/or maintainer of the code in question.

But yes, greybus is still in staging so it is ultimately your call.

I will see if I can find some spare cycles to start migrating at least
the core bits out of staging either way.

Johan

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-07-28  9:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-23  8:39 [PATCH] Staging: greybus: Match alignment with open parenthesis Shreeya Patel
2017-07-23  8:47 ` Johan Hovold
2017-07-23 12:10   ` Dan Carpenter
2017-07-23 12:54     ` Johan Hovold
2017-07-24  7:08       ` Dan Carpenter
2017-07-28  4:49         ` Greg KH
2017-07-28  9:01           ` Johan Hovold
2017-07-28  4:48 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).