[PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Hunter Chasens]

These two patches address some Errors, Warnings, and Checks reported by `./scripts/checkpatch.pl`

v1 -> v2
Separates the patch into a small patchset with the delimitation of syntactical styling vs whitespace styling. 
(I plan on sending to the maintainers if Outreach thinks this is an appropriate patch.)

Hunter Chasens (2):
  Staging: greybus: Fixed whitespace style issues
  Staging: greybus: Removed redundent 'int'

 .../Documentation/firmware/authenticate.c     |  4 +--
 .../greybus/Documentation/firmware/firmware.c | 34 +++++++++----------
 drivers/staging/greybus/arche-platform.c      |  2 +-
 .../staging/greybus/greybus_authentication.h  |  1 -
 drivers/staging/greybus/loopback.c            |  3 +-
 drivers/staging/greybus/tools/loopback_test.c | 16 +--------
 6 files changed, 22 insertions(+), 38 deletions(-)

-- 
2.37.3

[PATCH v2 1/2] Staging: greybus: Fixed whitespace style issues

[Hunter Chasens]

Fixed multiple whitespace and alignment code style issues.

Signed-off-by: Hunter Chasens <hunter.chasens18@ncf.edu>
---
 .../Documentation/firmware/authenticate.c     |  2 +-
 .../greybus/Documentation/firmware/firmware.c | 34 +++++++++----------
 drivers/staging/greybus/arche-platform.c      |  2 +-
 .../staging/greybus/greybus_authentication.h  |  1 -
 drivers/staging/greybus/loopback.c            |  3 +-
 drivers/staging/greybus/tools/loopback_test.c | 16 +--------
 6 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c
index 3d2c6f88a138..80f30c907905 100644
--- a/drivers/staging/greybus/Documentation/firmware/authenticate.c
+++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c
@@ -85,7 +85,7 @@ int main(int argc, char *argv[])
 	}
 
 	printf("Authenticated, result (%02x), sig-size (%02x)\n",
-		authenticate.result_code, authenticate.signature_size);
+	       authenticate.result_code, authenticate.signature_size);
 
 close_fd:
 	close(fd);
diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c b/drivers/staging/greybus/Documentation/firmware/firmware.c
index 765d69faa9cc..1955a97fc7c4 100644
--- a/drivers/staging/greybus/Documentation/firmware/firmware.c
+++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
@@ -47,13 +47,13 @@ static int update_intf_firmware(int fd)
 	ret = ioctl(fd, FW_MGMT_IOC_GET_INTF_FW, &intf_fw_info);
 	if (ret < 0) {
 		printf("Failed to get interface firmware version: %s (%d)\n",
-			fwdev, ret);
+		       fwdev, ret);
 		return -1;
 	}
 
 	printf("Interface Firmware tag (%s), major (%d), minor (%d)\n",
-		intf_fw_info.firmware_tag, intf_fw_info.major,
-		intf_fw_info.minor);
+	       intf_fw_info.firmware_tag, intf_fw_info.major,
+	       intf_fw_info.minor);
 
 	/* Try Interface Firmware load over Unipro */
 	printf("Loading Interface Firmware\n");
@@ -69,20 +69,20 @@ static int update_intf_firmware(int fd)
 	ret = ioctl(fd, FW_MGMT_IOC_INTF_LOAD_AND_VALIDATE, &intf_load);
 	if (ret < 0) {
 		printf("Failed to load interface firmware: %s (%d)\n", fwdev,
-			ret);
+		       ret);
 		return -1;
 	}
 
 	if (intf_load.status != GB_FW_U_LOAD_STATUS_VALIDATED &&
 	    intf_load.status != GB_FW_U_LOAD_STATUS_UNVALIDATED) {
 		printf("Load status says loading failed: %d\n",
-			intf_load.status);
+		       intf_load.status);
 		return -1;
 	}
 
 	printf("Interface Firmware (%s) Load done: major: %d, minor: %d, status: %d\n",
-		firmware_tag, intf_load.major, intf_load.minor,
-		intf_load.status);
+	       firmware_tag, intf_load.major, intf_load.minor,
+	       intf_load.status);
 
 	/* Initiate Mode-switch to the newly loaded firmware */
 	printf("Initiate Mode switch\n");
@@ -108,21 +108,21 @@ static int update_backend_firmware(int fd)
 	ret = ioctl(fd, FW_MGMT_IOC_GET_BACKEND_FW, &backend_fw_info);
 	if (ret < 0) {
 		printf("Failed to get backend firmware version: %s (%d)\n",
-			fwdev, ret);
+		       fwdev, ret);
 		return -1;
 	}
 
 	printf("Backend Firmware tag (%s), major (%d), minor (%d), status (%d)\n",
-		backend_fw_info.firmware_tag, backend_fw_info.major,
-		backend_fw_info.minor, backend_fw_info.status);
+	       backend_fw_info.firmware_tag, backend_fw_info.major,
+	       backend_fw_info.minor, backend_fw_info.status);
 
 	if (backend_fw_info.status == GB_FW_U_BACKEND_VERSION_STATUS_RETRY)
 		goto retry_fw_version;
 
-	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS)
-	    && (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
+	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS) &&
+	    (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
 		printf("Failed to get backend firmware version: %s (%d)\n",
-			fwdev, backend_fw_info.status);
+		       fwdev, backend_fw_info.status);
 		return -1;
 	}
 
@@ -148,10 +148,10 @@ static int update_backend_firmware(int fd)
 
 	if (backend_update.status != GB_FW_U_BACKEND_FW_STATUS_SUCCESS) {
 		printf("Load status says loading failed: %d\n",
-			backend_update.status);
+		       backend_update.status);
 	} else {
 		printf("Backend Firmware (%s) Load done: status: %d\n",
-				firmware_tag, backend_update.status);
+		       firmware_tag, backend_update.status);
 	}
 
 	return 0;
@@ -185,8 +185,8 @@ int main(int argc, char *argv[])
 		fw_timeout = strtoul(argv[4], &endptr, 10);
 
 	printf("Trying Firmware update: fwdev: %s, type: %s, tag: %s, timeout: %u\n",
-		fwdev, fw_update_type == 0 ? "interface" : "backend",
-		firmware_tag, fw_timeout);
+	       fwdev, fw_update_type == 0 ? "interface" : "backend",
+	       firmware_tag, fw_timeout);
 
 	printf("Opening %s firmware management device\n", fwdev);
 
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index fcbd5f71eff2..6f86b0141853 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -179,7 +179,7 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
 					arche_platform_set_wake_detect_state(arche_pdata,
-									     WD_STATE_COLDBOOT_TRIG);
+									    WD_STATE_COLDBOOT_TRIG);
 					spin_unlock_irqrestore(&arche_pdata->wake_lock,
 							       flags);
 					return IRQ_WAKE_THREAD;
diff --git a/drivers/staging/greybus/greybus_authentication.h b/drivers/staging/greybus/greybus_authentication.h
index 7edc7295b7ab..48b4a9794d3c 100644
--- a/drivers/staging/greybus/greybus_authentication.h
+++ b/drivers/staging/greybus/greybus_authentication.h
@@ -41,7 +41,6 @@
 #define CAP_AUTH_RESULT_CR_NO_KEY	0x03
 #define CAP_AUTH_RESULT_CR_SIG_FAIL	0x04
 
-
 /* IOCTL support */
 struct cap_ioc_get_endpoint_uid {
 	__u8			uid[8];
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 1a61fce98056..a6a975a442ad 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -588,8 +588,7 @@ static int gb_loopback_async_sink(struct gb_loopback *gb, u32 len)
 	return retval;
 }
 
-static int gb_loopback_async_transfer_complete(
-				struct gb_loopback_async_operation *op_async)
+static int gb_loopback_async_transfer_complete(struct gb_loopback_async_operation *op_async)
 {
 	struct gb_loopback *gb;
 	struct gb_operation *operation;
diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c
index 4c42e393cd3d..539c7030ffec 100644
--- a/drivers/staging/greybus/tools/loopback_test.c
+++ b/drivers/staging/greybus/tools/loopback_test.c
@@ -240,8 +240,6 @@ static void show_loopback_devices(struct loopback_test *t)
 	for (i = 0; i < t->device_count; i++)
 		printf("device[%d] = %s\n", i, t->devices[i].name);
 
-}
-
 int open_sysfs(const char *sys_pfx, const char *node, int flags)
 {
 	int fd;
@@ -274,7 +272,6 @@ float read_sysfs_float_fd(int fd, const char *sys_pfx, const char *node)
 	char buf[SYSFS_MAX_INT];
 
 	if (read(fd, buf, sizeof(buf)) < 0) {
-
 		fprintf(stderr, "unable to read from %s%s %s\n", sys_pfx, node,
 			strerror(errno));
 		close(fd);
@@ -367,7 +364,6 @@ static int get_results(struct loopback_test *t)
 			r->apbridge_unipro_latency_max - r->apbridge_unipro_latency_min;
 		r->gbphy_firmware_latency_jitter =
 			r->gbphy_firmware_latency_max - r->gbphy_firmware_latency_min;
-
 	}
 
 	/*calculate the aggregate results of all enabled devices */
@@ -407,9 +403,7 @@ static int get_results(struct loopback_test *t)
 			r->apbridge_unipro_latency_max - r->apbridge_unipro_latency_min;
 		r->gbphy_firmware_latency_jitter =
 			r->gbphy_firmware_latency_max - r->gbphy_firmware_latency_min;
-
 	}
-
 	return 0;
 }
 
@@ -537,7 +531,6 @@ static int log_results(struct loopback_test *t)
 			abort();
 		}
 
-	}
 	for (i = 0; i < t->device_count; i++) {
 		if (!device_enabled(t, i))
 			continue;
@@ -550,10 +543,8 @@ static int log_results(struct loopback_test *t)
 			if (ret == -1)
 				fprintf(stderr, "unable to write %d bytes to csv.\n", len);
 		}
-
 	}
 
-
 	if (t->aggregate_output) {
 		len = format_output(t, &t->aggregate_results, "aggregate",
 				    data, sizeof(data), &tm);
@@ -680,6 +671,7 @@ static int close_poll_files(struct loopback_test *t)
 
 	return 0;
 }
+
 static int is_complete(struct loopback_test *t)
 {
 	int iteration_count;
@@ -740,7 +732,6 @@ static int wait_for_complete(struct loopback_test *t)
 		ts = &t->poll_timeout;
 
 	while (1) {
-
 		ret = ppoll(t->fds, t->poll_count, ts, &mask_old);
 		if (ret <= 0) {
 			stop_tests(t);
@@ -780,7 +771,6 @@ static void prepare_devices(struct loopback_test *t)
 		if (t->stop_all || device_enabled(t, i))
 			write_sysfs_val(t->devices[i].sysfs_entry, "type", 0);
 
-
 	for (i = 0; i < t->device_count; i++) {
 		if (!device_enabled(t, i))
 			continue;
@@ -823,7 +813,6 @@ static int start(struct loopback_test *t)
 	return 0;
 }
 
-
 void loopback_run(struct loopback_test *t)
 {
 	int i;
@@ -852,7 +841,6 @@ void loopback_run(struct loopback_test *t)
 	if (ret)
 		goto err;
 
-
 	get_results(t);
 
 	log_results(t);
@@ -881,10 +869,8 @@ static int sanity_check(struct loopback_test *t)
 			fprintf(stderr, "Bad device mask %x\n", (1 << i));
 			return -1;
 		}
-
 	}
 
-
 	return 0;
 }
 
-- 
2.37.3

[PATCH v2 2/2] Staging: greybus: Removed redundent 'int'

[Hunter Chasens]

Removed redundent 'int' from phrase `unsigned long long int`.

Signed-off-by: Hunter Chasens <hunter.chasens18@ncf.edu>
---
 drivers/staging/greybus/Documentation/firmware/authenticate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c
index 80f30c907905..ba4b16b04557 100644
--- a/drivers/staging/greybus/Documentation/firmware/authenticate.c
+++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c
@@ -58,7 +58,7 @@ int main(int argc, char *argv[])
 		goto close_fd;
 	}
 
-	printf("UID received: 0x%llx\n", *(unsigned long long int *)(uid.uid));
+	printf("UID received: 0x%llx\n", *(unsigned long long *)(uid.uid));
 
 	/* Get certificate */
 	printf("Get IMS certificate\n");
-- 
2.37.3

Re: [PATCH v2 2/2] Staging: greybus: Removed redundent 'int'

[Julia Lawall]

On Mon, 10 Oct 2022, Hunter Chasens wrote:

> Removed redundent 'int' from phrase `unsigned long long int`.

For both patches, the subject line and the log message should be written
in the imperative.

Please see https://kernelnewbies.org/PatchPhilosophy

A good commit message should also say both what you are doing (briefly,
since the reader can also see that from the code) but also why.  Your
message expands on the subject, but doesn't answer the "why" part.

julia

>
> Signed-off-by: Hunter Chasens <hunter.chasens18@ncf.edu>
> ---
>  drivers/staging/greybus/Documentation/firmware/authenticate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c
> index 80f30c907905..ba4b16b04557 100644
> --- a/drivers/staging/greybus/Documentation/firmware/authenticate.c
> +++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c
> @@ -58,7 +58,7 @@ int main(int argc, char *argv[])
>  		goto close_fd;
>  	}
>
> -	printf("UID received: 0x%llx\n", *(unsigned long long int *)(uid.uid));
> +	printf("UID received: 0x%llx\n", *(unsigned long long *)(uid.uid));
>
>  	/* Get certificate */
>  	printf("Get IMS certificate\n");
> --
> 2.37.3
>
>
>

Re: [PATCH v2 1/2] Staging: greybus: Fixed whitespace style issues

[Julia Lawall]

On Mon, 10 Oct 2022, Hunter Chasens wrote:

> Fixed multiple whitespace and alignment code style issues.

You should also avoid the tempatation to say "fix".  Fix neither says what
you are doing nor why.  You are saying that you assume the result is
better, but the reader has to figure out all the details by themselves.

julia

>
> Signed-off-by: Hunter Chasens <hunter.chasens18@ncf.edu>
> ---
>  .../Documentation/firmware/authenticate.c     |  2 +-
>  .../greybus/Documentation/firmware/firmware.c | 34 +++++++++----------
>  drivers/staging/greybus/arche-platform.c      |  2 +-
>  .../staging/greybus/greybus_authentication.h  |  1 -
>  drivers/staging/greybus/loopback.c            |  3 +-
>  drivers/staging/greybus/tools/loopback_test.c | 16 +--------
>  6 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/staging/greybus/Documentation/firmware/authenticate.c b/drivers/staging/greybus/Documentation/firmware/authenticate.c
> index 3d2c6f88a138..80f30c907905 100644
> --- a/drivers/staging/greybus/Documentation/firmware/authenticate.c
> +++ b/drivers/staging/greybus/Documentation/firmware/authenticate.c
> @@ -85,7 +85,7 @@ int main(int argc, char *argv[])
>  	}
>
>  	printf("Authenticated, result (%02x), sig-size (%02x)\n",
> -		authenticate.result_code, authenticate.signature_size);
> +	       authenticate.result_code, authenticate.signature_size);
>
>  close_fd:
>  	close(fd);
> diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c b/drivers/staging/greybus/Documentation/firmware/firmware.c
> index 765d69faa9cc..1955a97fc7c4 100644
> --- a/drivers/staging/greybus/Documentation/firmware/firmware.c
> +++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
> @@ -47,13 +47,13 @@ static int update_intf_firmware(int fd)
>  	ret = ioctl(fd, FW_MGMT_IOC_GET_INTF_FW, &intf_fw_info);
>  	if (ret < 0) {
>  		printf("Failed to get interface firmware version: %s (%d)\n",
> -			fwdev, ret);
> +		       fwdev, ret);
>  		return -1;
>  	}
>
>  	printf("Interface Firmware tag (%s), major (%d), minor (%d)\n",
> -		intf_fw_info.firmware_tag, intf_fw_info.major,
> -		intf_fw_info.minor);
> +	       intf_fw_info.firmware_tag, intf_fw_info.major,
> +	       intf_fw_info.minor);
>
>  	/* Try Interface Firmware load over Unipro */
>  	printf("Loading Interface Firmware\n");
> @@ -69,20 +69,20 @@ static int update_intf_firmware(int fd)
>  	ret = ioctl(fd, FW_MGMT_IOC_INTF_LOAD_AND_VALIDATE, &intf_load);
>  	if (ret < 0) {
>  		printf("Failed to load interface firmware: %s (%d)\n", fwdev,
> -			ret);
> +		       ret);
>  		return -1;
>  	}
>
>  	if (intf_load.status != GB_FW_U_LOAD_STATUS_VALIDATED &&
>  	    intf_load.status != GB_FW_U_LOAD_STATUS_UNVALIDATED) {
>  		printf("Load status says loading failed: %d\n",
> -			intf_load.status);
> +		       intf_load.status);
>  		return -1;
>  	}
>
>  	printf("Interface Firmware (%s) Load done: major: %d, minor: %d, status: %d\n",
> -		firmware_tag, intf_load.major, intf_load.minor,
> -		intf_load.status);
> +	       firmware_tag, intf_load.major, intf_load.minor,
> +	       intf_load.status);
>
>  	/* Initiate Mode-switch to the newly loaded firmware */
>  	printf("Initiate Mode switch\n");
> @@ -108,21 +108,21 @@ static int update_backend_firmware(int fd)
>  	ret = ioctl(fd, FW_MGMT_IOC_GET_BACKEND_FW, &backend_fw_info);
>  	if (ret < 0) {
>  		printf("Failed to get backend firmware version: %s (%d)\n",
> -			fwdev, ret);
> +		       fwdev, ret);
>  		return -1;
>  	}
>
>  	printf("Backend Firmware tag (%s), major (%d), minor (%d), status (%d)\n",
> -		backend_fw_info.firmware_tag, backend_fw_info.major,
> -		backend_fw_info.minor, backend_fw_info.status);
> +	       backend_fw_info.firmware_tag, backend_fw_info.major,
> +	       backend_fw_info.minor, backend_fw_info.status);
>
>  	if (backend_fw_info.status == GB_FW_U_BACKEND_VERSION_STATUS_RETRY)
>  		goto retry_fw_version;
>
> -	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS)
> -	    && (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
> +	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS) &&
> +	    (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {
>  		printf("Failed to get backend firmware version: %s (%d)\n",
> -			fwdev, backend_fw_info.status);
> +		       fwdev, backend_fw_info.status);
>  		return -1;
>  	}
>
> @@ -148,10 +148,10 @@ static int update_backend_firmware(int fd)
>
>  	if (backend_update.status != GB_FW_U_BACKEND_FW_STATUS_SUCCESS) {
>  		printf("Load status says loading failed: %d\n",
> -			backend_update.status);
> +		       backend_update.status);
>  	} else {
>  		printf("Backend Firmware (%s) Load done: status: %d\n",
> -				firmware_tag, backend_update.status);
> +		       firmware_tag, backend_update.status);
>  	}
>
>  	return 0;
> @@ -185,8 +185,8 @@ int main(int argc, char *argv[])
>  		fw_timeout = strtoul(argv[4], &endptr, 10);
>
>  	printf("Trying Firmware update: fwdev: %s, type: %s, tag: %s, timeout: %u\n",
> -		fwdev, fw_update_type == 0 ? "interface" : "backend",
> -		firmware_tag, fw_timeout);
> +	       fwdev, fw_update_type == 0 ? "interface" : "backend",
> +	       firmware_tag, fw_timeout);
>
>  	printf("Opening %s firmware management device\n", fwdev);
>
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index fcbd5f71eff2..6f86b0141853 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -179,7 +179,7 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>  				if (arche_pdata->wake_detect_state !=
>  						WD_STATE_COLDBOOT_START) {
>  					arche_platform_set_wake_detect_state(arche_pdata,
> -									     WD_STATE_COLDBOOT_TRIG);
> +									    WD_STATE_COLDBOOT_TRIG);
>  					spin_unlock_irqrestore(&arche_pdata->wake_lock,
>  							       flags);
>  					return IRQ_WAKE_THREAD;
> diff --git a/drivers/staging/greybus/greybus_authentication.h b/drivers/staging/greybus/greybus_authentication.h
> index 7edc7295b7ab..48b4a9794d3c 100644
> --- a/drivers/staging/greybus/greybus_authentication.h
> +++ b/drivers/staging/greybus/greybus_authentication.h
> @@ -41,7 +41,6 @@
>  #define CAP_AUTH_RESULT_CR_NO_KEY	0x03
>  #define CAP_AUTH_RESULT_CR_SIG_FAIL	0x04
>
> -
>  /* IOCTL support */
>  struct cap_ioc_get_endpoint_uid {
>  	__u8			uid[8];
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 1a61fce98056..a6a975a442ad 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -588,8 +588,7 @@ static int gb_loopback_async_sink(struct gb_loopback *gb, u32 len)
>  	return retval;
>  }
>
> -static int gb_loopback_async_transfer_complete(
> -				struct gb_loopback_async_operation *op_async)
> +static int gb_loopback_async_transfer_complete(struct gb_loopback_async_operation *op_async)
>  {
>  	struct gb_loopback *gb;
>  	struct gb_operation *operation;
> diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c
> index 4c42e393cd3d..539c7030ffec 100644
> --- a/drivers/staging/greybus/tools/loopback_test.c
> +++ b/drivers/staging/greybus/tools/loopback_test.c
> @@ -240,8 +240,6 @@ static void show_loopback_devices(struct loopback_test *t)
>  	for (i = 0; i < t->device_count; i++)
>  		printf("device[%d] = %s\n", i, t->devices[i].name);
>
> -}
> -
>  int open_sysfs(const char *sys_pfx, const char *node, int flags)
>  {
>  	int fd;
> @@ -274,7 +272,6 @@ float read_sysfs_float_fd(int fd, const char *sys_pfx, const char *node)
>  	char buf[SYSFS_MAX_INT];
>
>  	if (read(fd, buf, sizeof(buf)) < 0) {
> -
>  		fprintf(stderr, "unable to read from %s%s %s\n", sys_pfx, node,
>  			strerror(errno));
>  		close(fd);
> @@ -367,7 +364,6 @@ static int get_results(struct loopback_test *t)
>  			r->apbridge_unipro_latency_max - r->apbridge_unipro_latency_min;
>  		r->gbphy_firmware_latency_jitter =
>  			r->gbphy_firmware_latency_max - r->gbphy_firmware_latency_min;
> -
>  	}
>
>  	/*calculate the aggregate results of all enabled devices */
> @@ -407,9 +403,7 @@ static int get_results(struct loopback_test *t)
>  			r->apbridge_unipro_latency_max - r->apbridge_unipro_latency_min;
>  		r->gbphy_firmware_latency_jitter =
>  			r->gbphy_firmware_latency_max - r->gbphy_firmware_latency_min;
> -
>  	}
> -
>  	return 0;
>  }
>
> @@ -537,7 +531,6 @@ static int log_results(struct loopback_test *t)
>  			abort();
>  		}
>
> -	}
>  	for (i = 0; i < t->device_count; i++) {
>  		if (!device_enabled(t, i))
>  			continue;
> @@ -550,10 +543,8 @@ static int log_results(struct loopback_test *t)
>  			if (ret == -1)
>  				fprintf(stderr, "unable to write %d bytes to csv.\n", len);
>  		}
> -
>  	}
>
> -
>  	if (t->aggregate_output) {
>  		len = format_output(t, &t->aggregate_results, "aggregate",
>  				    data, sizeof(data), &tm);
> @@ -680,6 +671,7 @@ static int close_poll_files(struct loopback_test *t)
>
>  	return 0;
>  }
> +
>  static int is_complete(struct loopback_test *t)
>  {
>  	int iteration_count;
> @@ -740,7 +732,6 @@ static int wait_for_complete(struct loopback_test *t)
>  		ts = &t->poll_timeout;
>
>  	while (1) {
> -
>  		ret = ppoll(t->fds, t->poll_count, ts, &mask_old);
>  		if (ret <= 0) {
>  			stop_tests(t);
> @@ -780,7 +771,6 @@ static void prepare_devices(struct loopback_test *t)
>  		if (t->stop_all || device_enabled(t, i))
>  			write_sysfs_val(t->devices[i].sysfs_entry, "type", 0);
>
> -
>  	for (i = 0; i < t->device_count; i++) {
>  		if (!device_enabled(t, i))
>  			continue;
> @@ -823,7 +813,6 @@ static int start(struct loopback_test *t)
>  	return 0;
>  }
>
> -
>  void loopback_run(struct loopback_test *t)
>  {
>  	int i;
> @@ -852,7 +841,6 @@ void loopback_run(struct loopback_test *t)
>  	if (ret)
>  		goto err;
>
> -
>  	get_results(t);
>
>  	log_results(t);
> @@ -881,10 +869,8 @@ static int sanity_check(struct loopback_test *t)
>  			fprintf(stderr, "Bad device mask %x\n", (1 << i));
>  			return -1;
>  		}
> -
>  	}
>
> -
>  	return 0;
>  }
>
> --
> 2.37.3
>
>
>

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Alison Schofield]

On Mon, Oct 10, 2022 at 03:51:57PM -0400, Hunter Chasens wrote:
> These two patches address some Errors, Warnings, and Checks reported by `./scripts/checkpatch.pl`
> 
> v1 -> v2
> Separates the patch into a small patchset with the delimitation of syntactical styling vs whitespace styling. 
> (I plan on sending to the maintainers if Outreach thinks this is an appropriate patch.)

Hi Hunter,
Please see the directions in the First Patch Tutorial and also repeated
in the mailing list message: 

>> You MUST send all patches to this mailing list AND to the appropriate
>> mailing lists and maintainers in get_maintainer.pl. The tutorial walks
>> you through how to use get_maintainer.pl.

We don't want to preview patches on the list. It saves you time and
reviewers time, as there on reviewers who will jump on your staging
patches that might not be on the Outreachy list. It's also the way
we develop - in the open.

So - in this case, you'll send out a v3 and in the changelog one of
the changed items needs to say - Expanded the 'CC list to include
maintainers.

Please do ask questions on the list - just do not post patches for
review that do not have the maintainers included.

Thanks!
Alison

> 
> Hunter Chasens (2):
>   Staging: greybus: Fixed whitespace style issues
>   Staging: greybus: Removed redundent 'int'
> 
>  .../Documentation/firmware/authenticate.c     |  4 +--
>  .../greybus/Documentation/firmware/firmware.c | 34 +++++++++----------
>  drivers/staging/greybus/arche-platform.c      |  2 +-
>  .../staging/greybus/greybus_authentication.h  |  1 -
>  drivers/staging/greybus/loopback.c            |  3 +-
>  drivers/staging/greybus/tools/loopback_test.c | 16 +--------
>  6 files changed, 22 insertions(+), 38 deletions(-)
> 
> -- 
> 2.37.3
> 
> 

Re: [PATCH v2 1/2] Staging: greybus: Fixed whitespace style issues

[Alison Schofield]

On Mon, Oct 10, 2022 at 03:51:58PM -0400, Hunter Chasens wrote:
> Fixed multiple whitespace and alignment code style issues.
> 
> Signed-off-by: Hunter Chasens <hunter.chasens18@ncf.edu>

Hi Hunter,

Checkpatch CHECK complained about this patch. It's not something you added,
but since you touched the line, it's going to show up when checkpatch
is run on your completed patch. 

Correcting this would be another patch in your set, since it's a
different type of cleanup - removing useless parenthesis.

CHECK: Unnecessary parentheses around 'backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS'
#92: FILE: drivers/staging/greybus/Documentation/firmware/firmware.c:122:
+	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS) &&
+	    (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {

CHECK: Unnecessary parentheses around 'backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE'
#92: FILE: drivers/staging/greybus/Documentation/firmware/firmware.c:122:
+	if ((backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_SUCCESS) &&
+	    (backend_fw_info.status != GB_FW_U_BACKEND_VERSION_STATUS_NOT_AVAILABLE)) {

--snip

Thanks,
Alison

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Hunter Chasens]

My apologies. I just felt very nervous about CCing the maintainers while the formatting of the patch and comments were still in the works. I'll be sure to do so in the v3. 
Thank you for your feedback. When running the get_maintainers script it shows the main LKML along with the mailing list for Staging. Should I also be CCing them, or just the indavidual maintainers? 

Re: [PATCH v2 1/2] Staging: greybus: Fixed whitespace style issues

[Hunter Chasens]

Thank you for your feedback, Alison. I was nervous to touch the parentheses as there was a macro involved. After reading your replies though I realise I was probably unreasonably concerned. I'll be sure to include that in the v3. Thank you!

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Alison Schofield]

On Mon, Oct 10, 2022 at 06:18:03PM -0400, Hunter Chasens wrote:
> My apologies. I just felt very nervous about CCing the maintainers while the formatting of the patch and comments were still in the works. I'll be sure to do so in the v3. 

above is one really one line, please break before 80.

> Thank you for your feedback. When running the get_maintainers script it shows the main LKML along with the mailing list for Staging. Should I also be CCing them, or just the indavidual maintainers? 

above is one really one line, please break before 80.

Hunter,

Understand the concern, and we try to mitigate it by working in the
staging/drivers directory during the contribution period. GregKH and
other maintainers with drivers in staging are expecting us :)  Also,
you'll meet some of the clean up pros of checkpatch, sparse, smatch,
and Coccinelle (you've already met Julia Lawall) working in this
space.

So - at the risk of overwhelming you on Day 1 - one more thing.
It seems you are sending plain text emails, but the text is wrapping
beyond 80 lines. It makes it difficult to cut and comment on a 
particular line. Although the kernel now accepts code lines that
exceeds 80 characters, we want to keep the patch log and email
conversations within the 80 char line limit.

Thanks,
Alison

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Alison Schofield]

Oops...not good to comment on someone's format and include typo's
yourself ;)  See my fixup below.


On Mon, Oct 10, 2022 at 04:53:40PM -0700, Alison Schofield wrote:
> On Mon, Oct 10, 2022 at 06:18:03PM -0400, Hunter Chasens wrote:
> > My apologies. I just felt very nervous about CCing the maintainers while the formatting of the patch and comments were still in the works. I'll be sure to do so in the v3. 
> 
> above is one really one line, please break before 80.
                      ^long
> 
> > Thank you for your feedback. When running the get_maintainers script it shows the main LKML along with the mailing list for Staging. Should I also be CCing them, or just the indavidual maintainers? 
> 
> above is one really one line, please break before 80.
                      ^long
> 
> Hunter,
> 
> Understand the concern, and we try to mitigate it by working in the
> staging/drivers directory during the contribution period. GregKH and
> other maintainers with drivers in staging are expecting us :)  Also,
> you'll meet some of the clean up pros of checkpatch, sparse, smatch,
> and Coccinelle (you've already met Julia Lawall) working in this
> space.
> 
> So - at the risk of overwhelming you on Day 1 - one more thing.
> It seems you are sending plain text emails, but the text is wrapping
> beyond 80 lines. It makes it difficult to cut and comment on a 
> particular line. Although the kernel now accepts code lines that
> exceeds 80 characters, we want to keep the patch log and email
> conversations within the 80 char line limit.
> 
> Thanks,
> Alison
> 

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Hunter Chasens]

I didn't know the 80-character limit applied to email as well! Thank you 
for telling me. I'm so used to auto-wrapping that it never occurred to me. 
I'm still getting used to mutt as well.

It is a bit overwhelming but it all seems very learnable with persistence and 
time. Sometimes it does feel like a game of whack-a-mole though. The moment I squash 
one issue is the moment my attention leaves another. :)

Re: [PATCH v2 0/2] Staging: greybus: Fixed whitespace/syntax style issues

[Julia Lawall]

On Mon, 10 Oct 2022, Hunter Chasens wrote:

> I didn't know the 80-character limit applied to email as well! Thank you
> for telling me. I'm so used to auto-wrapping that it never occurred to me.
> I'm still getting used to mutt as well.
>
> It is a bit overwhelming but it all seems very learnable with persistence and
> time. Sometimes it does feel like a game of whack-a-mole though. The moment I squash
> one issue is the moment my attention leaves another. :)

That's the purpose of sending the staging patches.  So you learn to do the
right thing without thinking.

julia