[PATCH v3 00/14] staging: unisys: cleanup series

[PATCH v3 00/14] staging: unisys: cleanup series

[Benjamin Romer]

This patch series cleans up all of the remaining checkpatch.pl issues in
the Unisys driver tree. The changes made were:

- The entire patch series has been rebased.
- A patch making parenthesis alignment changes to visornic_main.c was
  dropped from the series, because the original code was acceptable as is.
- Two patches were modified to move a fix to a NULL comparison into the
  correct patch.

Benjamin Romer (1):
  staging: unisys: remove unnecessary goto

Erik Arfvidson (13):
  staging: unisys: Fix guestlinuxdebug.h comments
  staging: unisys: fix comments for controlvmchannel.h
  staging: unisys: Fix NULL comparison vbusdeviceinfo.h
  staging: unisys: fix trailing comment in vbusdeviceinfo.h
  staging: unisys: fix comments in visorchipset
  staging: unisys: fix spacing in visorchipset.c
  staging: unisys: fix comparison to NULL in visorchipset.c
  staging: unisys: fix blank lines in visorchipset.c
  staging: unisys: fix spaces after cast visorchipset.c
  staging: unisys: fix comments in visorbus_main.c
  staging: unisys: fix block comments in ultrainputreport.h
  staging: unisys: fix comments in visornic_main.c
  staging: unisys: fix else statement in visornic_main.c

 drivers/staging/unisys/include/guestlinuxdebug.h   |  13 +-
 drivers/staging/unisys/visorbus/controlvmchannel.h | 237 +++++++++++----------
 drivers/staging/unisys/visorbus/vbusdeviceinfo.h   |   7 +-
 drivers/staging/unisys/visorbus/visorbus_main.c    |   6 +-
 drivers/staging/unisys/visorbus/visorchipset.c     |  50 ++---
 .../staging/unisys/visorinput/ultrainputreport.h   |  45 ++--
 drivers/staging/unisys/visornic/visornic_main.c    |  20 +-
 7 files changed, 197 insertions(+), 181 deletions(-)

-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 01/14] staging: unisys: Fix guestlinuxdebug.h comments

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch simply cleans up all checkpatch comment issues

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/include/guestlinuxdebug.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/include/guestlinuxdebug.h b/drivers/staging/unisys/include/guestlinuxdebug.h
index 82ee565..b81287f 100644
--- a/drivers/staging/unisys/include/guestlinuxdebug.h
+++ b/drivers/staging/unisys/include/guestlinuxdebug.h
@@ -17,9 +17,10 @@
 #define __GUESTLINUXDEBUG_H__
 
 /*
-* This file contains supporting interface for "vmcallinterface.h", particularly
-* regarding adding additional structure and functionality to linux
-* ISSUE_IO_VMCALL_POSTCODE_SEVERITY */
+ * This file contains supporting interface for "vmcallinterface.h", particularly
+ * regarding adding additional structure and functionality to linux
+ * ISSUE_IO_VMCALL_POSTCODE_SEVERITY
+ */
 
 /******* INFO ON ISSUE_POSTCODE_LINUX() BELOW *******/
 enum driver_pc {		/* POSTCODE driver identifier tuples */
@@ -133,9 +134,9 @@ enum event_pc {			/* POSTCODE event identifier tuples */
 
 #define POSTCODE_SEVERITY_ERR DIAG_SEVERITY_ERR
 #define POSTCODE_SEVERITY_WARNING DIAG_SEVERITY_WARNING
-#define POSTCODE_SEVERITY_INFO DIAG_SEVERITY_PRINT	/* TODO-> Info currently
-							 * doesn't show, so we
-							 * set info=warning */
+/* TODO-> Info currently doesn't show, so we set info=warning */
+#define POSTCODE_SEVERITY_INFO DIAG_SEVERITY_PRINT
+
 /* example call of POSTCODE_LINUX_2(VISOR_CHIPSET_PC, POSTCODE_SEVERITY_ERR);
  * Please also note that the resulting postcode is in hex, so if you are
  * searching for the __LINE__ number, convert it first to decimal.  The line
-- 
2.5.0

[PATCH v3 02/14] staging: unisys: fix comments for controlvmchannel.h

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch simply fixes all trailing */ by modifying the comments
structures while trying to reduce the total number of lines

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/controlvmchannel.h | 237 +++++++++++----------
 1 file changed, 119 insertions(+), 118 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/controlvmchannel.h b/drivers/staging/unisys/visorbus/controlvmchannel.h
index ec25366..03e36fb 100644
--- a/drivers/staging/unisys/visorbus/controlvmchannel.h
+++ b/drivers/staging/unisys/visorbus/controlvmchannel.h
@@ -55,22 +55,25 @@
 #define CONTROLVM_CRASHMSG_MAX		2
 
 struct spar_segment_state  {
-	u16 enabled:1;		/* Bit 0: May enter other states */
-	u16 active:1;		/* Bit 1: Assigned to active partition */
-	u16 alive:1;		/* Bit 2: Configure message sent to
-				 * service/server */
-	u16 revoked:1;		/* Bit 3: similar to partition state
-				 * ShuttingDown */
-	u16 allocated:1;	/* Bit 4: memory (device/port number)
-				 * has been selected by Command */
-	u16 known:1;		/* Bit 5: has been introduced to the
-				 * service/guest partition */
-	u16 ready:1;		/* Bit 6: service/Guest partition has
-				 * responded to introduction */
-	u16 operating:1;	/* Bit 7: resource is configured and
-				 * operating */
-	/* Note: don't use high bit unless we need to switch to ushort
-	 * which is non-compliant */
+	/* Bit 0: May enter other states */
+	u16 enabled:1;
+	/* Bit 1: Assigned to active partition */
+	u16 active:1;
+	/* Bit 2: Configure message sent to service/server */
+	u16 alive:1;
+	/* Bit 3: similar to partition state ShuttingDown */
+	u16 revoked:1;
+	/* Bit 4: memory (device/port number) has been selected by Command */
+	u16 allocated:1;
+	/* Bit 5: has been introduced to the service/guest partition */
+	u16 known:1;
+	/* Bit 6: service/Guest partition has responded to introduction */
+	u16 ready:1;
+	/* Bit 7: resource is configured and operating */
+	u16 operating:1;
+/* Note: don't use high bit unless we need to switch to ushort
+ * which is non-compliant
+ */
 };
 
 static const struct spar_segment_state segment_state_running = {
@@ -177,53 +180,53 @@ struct controlvm_message_header  {
 	/* For requests, indicates the message type. */
 	/* For responses, indicates the type of message we are responding to. */
 
-	u32 message_size;	/* Includes size of this struct + size
-				 * of message */
-	u32 segment_index;	/* Index of segment containing Vm
-				 * message/information */
-	u32 completion_status;	/* Error status code or result of
-				 * message completion */
+	/* Includes size of this struct + size of message */
+	u32 message_size;
+	/* Index of segment containing Vm message/information */
+	u32 segment_index;
+	/* Error status code or result of  message completion */
+	u32 completion_status;
 	struct  {
-		u32 failed:1;		   /* =1 in a response to * signify
-					    * failure */
-		u32 response_expected:1;   /* =1 in all messages that expect a
-					    * response (Control ignores this
-					    * bit) */
-		u32 server:1;		   /* =1 in all bus & device-related
-					    * messages where the message
-					    * receiver is to act as the bus or
-					    * device server */
-		u32 test_message:1;	   /* =1 for testing use only
-					    * (Control and Command ignore this
-					    * bit) */
-		u32 partial_completion:1;  /* =1 if there are forthcoming
-					    * responses/acks associated
-					    * with this message */
-		u32 preserve:1;		   /* =1 this is to let us know to
-					    * preserve channel contents
-					    * (for running guests)*/
-		u32 writer_in_diag:1;	   /* =1 the DiagWriter is active in the
-					    * Diagnostic Partition*/
+		/* =1 in a response to signify failure */
+		u32 failed:1;
+		/* =1 in all messages that expect a response */
+		u32 response_expected:1;
+		/* =1 in all bus & device-related messages where the message
+		 * receiver is to act as the bus or device server
+		 */
+		u32 server:1;
+		/* =1 for testing use only (Control and Command ignore this */
+		u32 test_message:1;
+		/* =1 if there are forthcoming responses/acks associated
+		 * with this message
+		 */
+		u32 partial_completion:1;
+		/* =1 this is to let us know to preserve channel contents */
+		u32 preserve:1;
+		/* =1 the DiagWriter is active in the Diagnostic Partition */
+		u32 writer_in_diag:1;
 	} flags;
-	u32 reserved;		/* Natural alignment */
-	u64 message_handle;	/* Identifies the particular message instance,
-				 * and is used to match particular */
+	/* Natural alignment */
+	u32 reserved;
+	/* Identifies the particular message instance */
+	u64 message_handle;
 	/* request instances with the corresponding response instance. */
-	u64 payload_vm_offset;	/* Offset of payload area from start of this
-				 * instance of ControlVm segment */
-	u32 payload_max_bytes;	/* Maximum bytes allocated in payload
-				 * area of ControlVm segment */
-	u32 payload_bytes;	/* Actual number of bytes of payload
-				 * area to copy between IO/Command; */
+	/* Offset of payload area from start of this instance */
+	u64 payload_vm_offset;
+	/* Maximum bytes allocated in payload area of ControlVm segment */
+	u32 payload_max_bytes;
+	/* Actual number of bytes of payload area to copy between IO/Command */
+	u32 payload_bytes;
 	/* if non-zero, there is a payload to copy. */
 };
 
 struct controlvm_packet_device_create  {
 	u32 bus_no;		/* bus # (0..n-1) from the msg receiver's end */
 	u32 dev_no;		/* bus-relative (0..n-1) device number */
-	u64 channel_addr;	/* Guest physical address of the channel, which
-				 * can be dereferenced by the receiver of this
-				 * ControlVm command */
+	/* Guest physical address of the channel, which can be dereferenced by
+	 * the receiver of this ControlVm command
+	 */
+	u64 channel_addr;
 	u64 channel_bytes;	/* specifies size of the channel in bytes */
 	uuid_le data_type_uuid;	/* specifies format of data in channel */
 	uuid_le dev_inst_uuid;	/* instance guid for the device */
@@ -231,8 +234,8 @@ struct controlvm_packet_device_create  {
 };	/* for CONTROLVM_DEVICE_CREATE */
 
 struct controlvm_packet_device_configure  {
-	u32 bus_no;	      /* bus # (0..n-1) from the msg
-			       * receiver's perspective */
+	/* bus # (0..n-1) from the msg receiver's perspective */
+	u32 bus_no;
 	/* Control uses header SegmentIndex field to access bus number... */
 	u32 dev_no;	      /* bus-relative (0..n-1) device number */
 } ;	/* for CONTROLVM_DEVICE_CONFIGURE */
@@ -251,50 +254,50 @@ struct controlvm_message_device_configure  {
 struct controlvm_message_packet  {
 	union  {
 		struct  {
-			u32 bus_no;	/* bus # (0..n-1) from the msg
-					 * receiver's perspective */
-			u32 dev_count;	/* indicates the max number of
-					 * devices on this bus */
-			u64 channel_addr;	/* Guest physical address of
-						 * the channel, which can be
-						 * dereferenced by the receiver
-						 * of this ControlVm command */
+	/* bus # (0..n-1) from the msg receiver's perspective */
+			u32 bus_no;
+	/* indicates the max number of devices on this bus */
+			u32 dev_count;
+	/* Guest physical address of the channel, which can be
+	 * dereferenced by the receiver of this ControlVm command
+	 */
+			u64 channel_addr;
 			u64 channel_bytes;	/* size of the channel */
-			uuid_le bus_data_type_uuid;	/* indicates format of
-							 * data in bus channel*/
+	/* indicates format of data in bus channel*/
+			uuid_le bus_data_type_uuid;
 			uuid_le bus_inst_uuid;	/* instance uuid for the bus */
 		} create_bus;	/* for CONTROLVM_BUS_CREATE */
 		struct  {
-			u32 bus_no;	/* bus # (0..n-1) from the msg
-					 * receiver's perspective */
+	/* bus # (0..n-1) from the msg receiver's perspective */
+			u32 bus_no;
 			u32 reserved;	/* Natural alignment purposes */
 		} destroy_bus;	/* for CONTROLVM_BUS_DESTROY */
 		struct  {
-			u32 bus_no;	/* bus # (0..n-1) from the receiver's
-					 * perspective */
+	/* bus # (0..n-1) from the receiver's perspective */
+			u32 bus_no;
 			u32 reserved1;	/* for alignment purposes */
-			u64 guest_handle;	/* This is used to convert
-						 * guest physical address to
-						 * physical address */
+	/* This is used to convert guest physical address to physical address */
+			u64 guest_handle;
 			u64 recv_bus_irq_handle;
 				/* specifies interrupt info. It is used by SP
 				 * to register to receive interrupts from the
 				 * CP. This interrupt is used for bus level
 				 * notifications.  The corresponding
-				 * sendBusInterruptHandle is kept in CP. */
+				 * sendBusInterruptHandle is kept in CP.
+				 */
 		} configure_bus;	/* for CONTROLVM_BUS_CONFIGURE */
 		/* for CONTROLVM_DEVICE_CREATE */
 		struct controlvm_packet_device_create create_device;
 		struct  {
-			u32 bus_no;	/* bus # (0..n-1) from the msg
-					 * receiver's perspective */
+		/* bus # (0..n-1) from the msg receiver's perspective */
+			u32 bus_no;
 			u32 dev_no;	/* bus-relative (0..n-1) device # */
 		} destroy_device;	/* for CONTROLVM_DEVICE_DESTROY */
 		/* for CONTROLVM_DEVICE_CONFIGURE */
 		struct controlvm_packet_device_configure configure_device;
 		struct  {
-			u32 bus_no;	/* bus # (0..n-1) from the msg
-					 * receiver's perspective */
+		/* bus # (0..n-1) from the msg receiver's perspective */
+			u32 bus_no;
 			u32 dev_no;	/* bus-relative (0..n-1) device # */
 		} reconfigure_device;	/* for CONTROLVM_DEVICE_RECONFIGURE */
 		struct  {
@@ -307,8 +310,8 @@ struct controlvm_message_packet  {
 			u32 dev_no;
 			struct spar_segment_state state;
 			struct  {
-				u32 phys_device:1;	/* =1 if message is for
-							 * a physical device */
+				/* =1 if message is for a physical device */
+				u32 phys_device:1;
 			} flags;
 			u8 reserved[2];	/* Natural alignment purposes */
 		} device_change_state;	/* for CONTROLVM_DEVICE_CHANGESTATE */
@@ -320,9 +323,10 @@ struct controlvm_message_packet  {
 		} device_change_state_event;
 			/* for CONTROLVM_DEVICE_CHANGESTATE_EVENT */
 		struct  {
-			u32 bus_count;	/* indicates the max number of busses */
-			u32 switch_count; /* indicates the max number of
-					   * switches if a service partition */
+			/* indicates the max number of busses */
+			u32 bus_count;
+			/* indicates the max number of switches */
+			u32 switch_count;
 			enum ultra_chipset_feature features;
 			u32 platform_number;	/* Platform Number */
 		} init_chipset;	/* for CONTROLVM_CHIPSET_INIT */
@@ -330,11 +334,12 @@ struct controlvm_message_packet  {
 			u32 options;	/* reserved */
 			u32 test;	/* bit 0 set to run embedded selftest */
 		} chipset_selftest;	/* for CONTROLVM_CHIPSET_SELFTEST */
-		u64 addr;	/* a physical address of something, that can be
-				 * dereferenced by the receiver of this
-				 * ControlVm command (depends on command id) */
-		u64 handle;	/* a handle of something (depends on command
-				 * id) */
+		/* a physical address of something, that can be dereferenced
+		 * by the receiver of this ControlVm command
+		 */
+		u64 addr;
+		/* a handle of something (depends on command id) */
+		u64 handle;
 	};
 };
 
@@ -357,8 +362,8 @@ struct spar_controlvm_channel_protocol {
 	u64 gp_nvram;	/* guest phys addr of NVRAM channel */
 	u64 request_payload_offset;	/* Offset to request payload area */
 	u64 event_payload_offset;	/* Offset to event payload area */
-	u32 request_payload_bytes;	/* Bytes available in request payload
-					 * area */
+	/* Bytes available in request payload area */
+	u32 request_payload_bytes;
 	u32 event_payload_bytes;/* Bytes available in event payload area */
 	u32 control_channel_bytes;
 	u32 nvram_channel_bytes;	/* Bytes in PartitionNvram segment */
@@ -384,41 +389,37 @@ struct spar_controlvm_channel_protocol {
 	u64 virtual_guest_image_size;
 	u64 prototype_control_channel_offset;
 	u64 virtual_guest_partition_handle;
-
-	u16 restore_action;	/* Restore Action field to restore the guest
-				 * partition */
-	u16 dump_action;	/* For Windows guests it shows if the visordisk
-				 * is running in dump mode */
+	/* Restore Action field to restore the guest partition */
+	u16 restore_action;
+	/* For Windows guests it shows if the visordisk is in dump mode */
+	u16 dump_action;
 	u16 nvram_fail_count;
 	u16 saved_crash_message_count;	/* = CONTROLVM_CRASHMSG_MAX */
-	u32 saved_crash_message_offset;	/* Offset to request payload area needed
-					 * for crash dump */
-	u32 installation_error;	/* Type of error encountered during
-				 * installation */
+	/* Offset to request payload area needed for crash dump */
+	u32 saved_crash_message_offset;
+	/* Type of error encountered during installation */
+	u32 installation_error;
 	u32 installation_text_id;	/* Id of string to display */
-	u16 installation_remaining_steps;/* Number of remaining installation
-					  * steps (for progress bars) */
-	u8 tool_action;		/* ULTRA_TOOL_ACTIONS Installation Action
-				 * field */
+	/* Number of remaining installation  steps (for progress bars) */
+	u16 installation_remaining_steps;
+	/* ULTRA_TOOL_ACTIONS Installation Action field */
+	u8 tool_action;
 	u8 reserved;		/* alignment */
 	struct efi_spar_indication efi_spar_ind;
 	struct efi_spar_indication efi_spar_ind_supported;
 	u32 sp_reserved;
-	u8 reserved2[28];	/* Force signals to begin on 128-byte cache
-				 * line */
-	struct signal_queue_header request_queue;/* Service or guest partition
-						  * uses this queue to send
-						  * requests to Control */
-	struct signal_queue_header response_queue;/* Control uses this queue to
-						   * respond to service or guest
-						   * partition requests */
-	struct signal_queue_header event_queue;	/* Control uses this queue to
-						 * send events to service or
-						 * guest partition */
-	struct signal_queue_header event_ack_queue;/* Service or guest partition
-						    * uses this queue to ack
-						    * Control events */
-
+	/* Force signals to begin on 128-byte cache line */
+	u8 reserved2[28];
+	/* guest partition uses this queue to send requests to Control */
+	struct signal_queue_header request_queue;
+	/* Control uses this queue to respond to service or guest
+	 * partition requests
+	 */
+	struct signal_queue_header response_queue;
+	/* Control uses this queue to send events to guest partition */
+	struct signal_queue_header event_queue;
+	/* Service or guest partition  uses this queue to ack Control events */
+	struct signal_queue_header event_ack_queue;
 	 /* Request fixed-size message pool - does not include payload */
 	 struct controlvm_message request_msg[CONTROLVM_MESSAGE_MAX];
 
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 03/14] staging: unisys: Fix NULL comparison vbusdeviceinfo.h

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patches resolves the NULL comparison checkpatch warnings

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/vbusdeviceinfo.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
index f59fd8a..3216975 100644
--- a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
+++ b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
@@ -62,7 +62,7 @@ vbuschannel_sanitize_buffer(char *p, int remain, char *src, int srcmax)
 					p++;
 					remain--;
 					chars++;
-				} else if (p == NULL) {
+				} else if (!p) {
 					chars++;
 				}
 				nonprintable_streak = 0;
@@ -72,7 +72,7 @@ vbuschannel_sanitize_buffer(char *p, int remain, char *src, int srcmax)
 				p++;
 				remain--;
 				chars++;
-			} else if (p == NULL) {
+			} else if (!p) {
 				chars++;
 			}
 		} else {
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 04/14] staging: unisys: fix trailing comment in vbusdeviceinfo.h

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

Fixes checkpatch trailing */ comment in vbusdeviceinfo.h

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/vbusdeviceinfo.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
index 3216975..abdab4a 100644
--- a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
+++ b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
@@ -124,7 +124,8 @@ vbuschannel_itoa(char *p, int remain, int num)
 	}
 	if (remain < digits) {
 		/* not enough room left at <p> to hold number, so fill with
-		 * '?' */
+		 * '?'
+		 */
 		for (i = 0; i < remain; i++, p++)
 			*p = '?';
 		return remain;
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 05/14] staging: unisys: fix comments in visorchipset

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes all the Block comments by using a trailing */
on a separate line

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index f4c37da..c3d46aa 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -86,8 +86,8 @@ visorchipset_release(struct inode *inode, struct file *file)
 */
 #define MIN_IDLE_SECONDS 10
 static unsigned long poll_jiffies = POLLJIFFIES_CONTROLVMCHANNEL_FAST;
-static unsigned long most_recent_message_jiffies;	/* when we got our last
-						 * controlvm message */
+/* when we got our last controlvm message */
+static unsigned long most_recent_message_jiffies;
 static int visorbusregistered;
 
 #define MAX_CHIPSET_EVENTS 2
@@ -120,7 +120,8 @@ static struct visorchannel *controlvm_channel;
 struct visor_controlvm_payload_info {
 	u8 *ptr;		/* pointer to base address of payload pool */
 	u64 offset;		/* offset from beginning of controlvm
-				 * channel to beginning of payload * pool */
+				 * channel to beginning of payload * pool
+				 */
 	u32 bytes;		/* number of bytes in payload pool */
 };
 
@@ -184,7 +185,8 @@ struct putfile_request {
 	 * - this list is added to when controlvm messages come in that supply
 	 * file data
 	 * - this list is removed from via the hotplug program that is actually
-	 * consuming these buffers to write as file data */
+	 * consuming these buffers to write as file data
+	 */
 	struct list_head input_buffer_list;
 	spinlock_t req_list_lock;	/* lock for input_buffer_list */
 
@@ -788,13 +790,15 @@ chipset_init(struct controlvm_message *inmsg)
 	POSTCODE_LINUX_2(CHIPSET_INIT_EXIT_PC, POSTCODE_SEVERITY_INFO);
 
 	/* Set features to indicate we support parahotplug (if Command
-	 * also supports it). */
+	 * also supports it).
+	 */
 	features =
 	    inmsg->cmd.init_chipset.
 	    features & ULTRA_CHIPSET_FEATURE_PARA_HOTPLUG;
 
 	/* Set the "reply" bit so Command knows this is a
-	 * features-aware driver. */
+	 * features-aware driver.
+	 */
 	features |= ULTRA_CHIPSET_FEATURE_REPLY;
 
 cleanup:
-- 
2.5.0

[PATCH v3 06/14] staging: unisys: fix spacing in visorchipset.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following checkpatch warning:
spaces preferred around that ‘*’ or ‘|’

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: address Sudip's comment about a change being made to a NULL comparison
    that should have been made in a previous patch.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index c3d46aa..085d19a 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -47,7 +47,6 @@
 
 #define VISORCHIPSET_MMAP_CONTROLCHANOFFSET	0x00000000
 
-
 #define UNISYS_SPAR_LEAF_ID 0x40000000
 
 /* The s-Par leaf ID returns "UnisysSpar64" encoded across ebx, ecx, edx */
@@ -379,7 +378,7 @@ parser_init_byte_stream(u64 addr, u32 bytes, bool local, bool *retry)
 		rc = NULL;
 		goto cleanup;
 	}
-	ctx = kzalloc(allocbytes, GFP_KERNEL|__GFP_NORETRY);
+	ctx = kzalloc(allocbytes, GFP_KERNEL | __GFP_NORETRY);
 	if (!ctx) {
 		if (retry)
 			*retry = true;
@@ -522,7 +521,7 @@ parser_string_get(struct parser_context *ctx)
 		}
 	if (value_length < 0)	/* '\0' was not included in the length */
 		value_length = nscan;
-	value = kmalloc(value_length + 1, GFP_KERNEL|__GFP_NORETRY);
+	value = kmalloc(value_length + 1, GFP_KERNEL | __GFP_NORETRY);
 	if (value == NULL)
 		return NULL;
 	if (value_length > 0)
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 07/14] staging: unisys: fix comparison to NULL in visorchipset.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following type of check patch warnings:
Comparison to NULL could be written

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: address a comment from Sudip about a change to a NULL comparison that
    was made in a later patch, that should have been part of this one.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 085d19a..ab8f9dd 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -438,7 +438,7 @@ parser_id_get(struct parser_context *ctx)
 {
 	struct spar_controlvm_parameters_header *phdr = NULL;
 
-	if (ctx == NULL)
+	if (!ctx)
 		return NULL_UUID_LE;
 	phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
 	return phdr->id;
@@ -461,7 +461,7 @@ parser_param_start(struct parser_context *ctx,
 {
 	struct spar_controlvm_parameters_header *phdr = NULL;
 
-	if (ctx == NULL)
+	if (!ctx)
 		goto Away;
 	phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
 	switch (which_string) {
@@ -522,7 +522,7 @@ parser_string_get(struct parser_context *ctx)
 	if (value_length < 0)	/* '\0' was not included in the length */
 		value_length = nscan;
 	value = kmalloc(value_length + 1, GFP_KERNEL | __GFP_NORETRY);
-	if (value == NULL)
+	if (!value)
 		return NULL;
 	if (value_length > 0)
 		memcpy(value, pscan, value_length);
@@ -875,7 +875,7 @@ bus_responder(enum controlvm_id cmd_id,
 	      struct controlvm_message_header *pending_msg_hdr,
 	      int response)
 {
-	if (pending_msg_hdr == NULL)
+	if (!pending_msg_hdr)
 		return;		/* no controlvm response needed */
 
 	if (pending_msg_hdr->id != (u32)cmd_id)
@@ -893,7 +893,7 @@ device_changestate_responder(enum controlvm_id cmd_id,
 	u32 bus_no = p->chipset_bus_no;
 	u32 dev_no = p->chipset_dev_no;
 
-	if (p->pending_msg_hdr == NULL)
+	if (!p->pending_msg_hdr)
 		return;		/* no controlvm response needed */
 	if (p->pending_msg_hdr->id != cmd_id)
 		return;
@@ -914,7 +914,7 @@ device_responder(enum controlvm_id cmd_id,
 		 struct controlvm_message_header *pending_msg_hdr,
 		 int response)
 {
-	if (pending_msg_hdr == NULL)
+	if (!pending_msg_hdr)
 		return;		/* no controlvm response needed */
 
 	if (pending_msg_hdr->id != (u32)cmd_id)
@@ -1180,7 +1180,7 @@ bus_configure(struct controlvm_message *inmsg,
 		POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
 				 POSTCODE_SEVERITY_ERR);
 		rc = -CONTROLVM_RESP_ERROR_BUS_INVALID;
-	} else if (bus_info->pending_msg_hdr != NULL) {
+	} else if (bus_info->pending_msg_hdr) {
 		POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, bus_no,
 				 POSTCODE_SEVERITY_ERR);
 		rc = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 08/14] staging: unisys: fix blank lines in visorchipset.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

this patch removes the following checkpatch warnings:
	please use a blank line after …
	Please don’t use multiple blank lines

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index ab8f9dd..8d672c7 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -353,7 +353,6 @@ static void controlvm_respond_physdev_changestate(
 		struct controlvm_message_header *msg_hdr, int response,
 		struct spar_segment_state state);
 
-
 static void parser_done(struct parser_context *ctx);
 
 static struct parser_context *
@@ -530,7 +529,6 @@ parser_string_get(struct parser_context *ctx)
 	return value;
 }
 
-
 static ssize_t toolaction_show(struct device *dev,
 			       struct device_attribute *attr,
 			       char *buf)
@@ -707,6 +705,7 @@ static int match_visorbus_dev_by_id(struct device *dev, void *data)
 
 	return 0;
 }
+
 struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
 					       struct visor_device *from)
 {
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 09/14] staging: unisys: fix spaces after cast visorchipset.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes checkpatch's no space is necessary after a cast

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 8d672c7..6ad03a5 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -397,7 +397,7 @@ parser_init_byte_stream(u64 addr, u32 bytes, bool local, bool *retry)
 			rc = NULL;
 			goto cleanup;
 		}
-		p = __va((unsigned long) (addr));
+		p = __va((unsigned long)(addr));
 		memcpy(ctx->data, p, bytes);
 	} else {
 		void *mapping;
@@ -525,7 +525,7 @@ parser_string_get(struct parser_context *ctx)
 		return NULL;
 	if (value_length > 0)
 		memcpy(value, pscan, value_length);
-	((u8 *) (value))[value_length] = '\0';
+	((u8 *)(value))[value_length] = '\0';
 	return value;
 }
 
@@ -815,7 +815,7 @@ controlvm_init_response(struct controlvm_message *msg,
 	msg->hdr.payload_max_bytes = 0;
 	if (response < 0) {
 		msg->hdr.flags.failed = 1;
-		msg->hdr.completion_status = (u32) (-response);
+		msg->hdr.completion_status = (u32)(-response);
 	}
 }
 
-- 
2.5.0

[PATCH v3 10/14] staging: unisys: fix comments in visorbus_main.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following types of check patch warnings:
	Block comments use a trailing */ on a separate line
	Comment alignments

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index eac97d2..a301385 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -1371,9 +1371,9 @@ pause_state_change_complete(struct visor_device *dev, int status)
 		return;
 
 	/* Notify the chipset driver that the pause is complete, which
-	* will presumably want to send some sort of response to the
-	* initiator.
-	*/
+	 * will presumably want to send some sort of response to the
+	 * initiator.
+	 */
 	(*chipset_responders.device_pause) (dev, status);
 }
 
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 11/14] staging: unisys: fix block comments in ultrainputreport.h

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following types of check patch warnings:
Block comments use * on subsequent lines
Block comments use a trailing */ on a separate line

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 .../staging/unisys/visorinput/ultrainputreport.h   | 45 +++++++++++++---------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/ultrainputreport.h b/drivers/staging/unisys/visorinput/ultrainputreport.h
index 3e6a52f..1bc3d20 100644
--- a/drivers/staging/unisys/visorinput/ultrainputreport.h
+++ b/drivers/staging/unisys/visorinput/ultrainputreport.h
@@ -29,33 +29,40 @@ enum ultra_inputaction {
 	inputaction_mouse_button_up = 3, /* arg1: 1=left,2=center,3=right */
 	inputaction_mouse_button_click = 4, /* arg1: 1=left,2=center,3=right */
 	inputaction_mouse_button_dclick = 5, /* arg1: 1=left,2=center,
-						3=right */
+					      * 3=right
+					      */
 	inputaction_wheel_rotate_away = 6, /* arg1: wheel rotation away from
-					      user */
+					    * user
+					    */
 	inputaction_wheel_rotate_toward = 7, /* arg1: wheel rotation toward
-						user */
+					      * user
+					      */
 	inputaction_set_max_xy = 8,	/* set screen maxXY; arg1=x, arg2=y */
 	inputaction_key_down = 64,	/* arg1: scancode, as follows:
-					   If arg1 <= 0xff, it's a 1-byte
-					   scancode and arg1 is that scancode.
-					   If arg1 > 0xff, it's a 2-byte
-					   scanecode, with the 1st byte in the
-					   low 8 bits, and the 2nd byte in the
-					   high 8 bits.  E.g., the right ALT key
-					   would appear as x'38e0'. */
+					 * If arg1 <= 0xff, it's a 1-byte
+					 * scancode and arg1 is that scancode.
+					 * If arg1 > 0xff, it's a 2-byte
+					 * scanecode, with the 1st byte in the
+					 * low 8 bits, and the 2nd byte in the
+					 * high 8 bits.  E.g., the right ALT key
+					 * would appear as x'38e0'.
+					 */
 	inputaction_key_up = 65,	/* arg1: scancode (in same format as
-					   inputaction_keyDown) */
+					 * inputaction_keyDown)
+					 */
 	inputaction_set_locking_key_state = 66,
 					/* arg1: scancode (in same format
-						 as inputaction_keyDown);
-						 MUST refer to one of the
-						 locking keys, like capslock,
-						 numlock, or scrolllock
-					   arg2: 1 iff locking key should be
-						 in the LOCKED position
-						 (e.g., light is ON) */
+					 *	 as inputaction_keyDown);
+					 *	 MUST refer to one of the
+					 *	 locking keys, like capslock,
+					 *	 numlock, or scrolllock
+					 * arg2: 1 iff locking key should be
+					 *	 in the LOCKED position
+					 *	 (e.g., light is ON)
+					 */
 	inputaction_key_down_up = 67,	/* arg1: scancode (in same format
-						 as inputaction_keyDown) */
+					 *	 as inputaction_keyDown)
+					 */
 	inputaction_last
 };
 
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 12/14] staging: unisys: fix comments in visornic_main.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following types of check patch warnings:
	Block comments use * on subsequent lines
	Block comments use a trailing */ on a separate line

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visornic/visornic_main.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 9b3eb95..3da849a 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -1219,8 +1219,9 @@ visornic_rx(struct uiscmdrsp *cmdrsp)
 		/* length rcvd is greater than firstfrag in this skb rcv buf  */
 		skb->tail += RCVPOST_BUF_SIZE;	/* amount in skb->data */
 		skb->data_len = skb->len - RCVPOST_BUF_SIZE;	/* amount that
-								   will be in
-								   frag_list */
+								 *  will be in
+								 * frag_list
+								 */
 	} else {
 		/* data fits in this skb - no chaining - do
 		 * PRECAUTIONARY check
@@ -1316,12 +1317,14 @@ visornic_rx(struct uiscmdrsp *cmdrsp)
 				}
 				if (found_mc)
 					break;	/* accept packet, dest
-						   matches a multicast
-						   address */
+						 * matches a multicast
+						 * address
+						 */
 			}
 		} else if (skb->pkt_type == PACKET_HOST) {
 			break;	/* accept packet, h_dest must match vnic
-				   mac address */
+				 *  mac address
+				 */
 		} else if (skb->pkt_type == PACKET_OTHERHOST) {
 			/* something is not right */
 			dev_err(&devdata->netdev->dev,
@@ -1619,6 +1622,9 @@ service_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata,
 	struct net_device *netdev;
 
 	while (*rx_work_done < budget) {
+	/* TODO: CLIENT ACQUIRE -- Don't really need this at the
+	 * moment
+	 */
 		if (!visorchannel_signalremove(devdata->dev->visorchannel,
 					       IOCHAN_FROM_IOPART,
 					       cmdrsp))
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 13/14] staging: unisys: fix else statement in visornic_main.c

[Benjamin Romer]

From: Erik Arfvidson <erik.arfvidson@unisys.com>

This patch fixes the following types of check patch warnings:
else is not generally useful after a break or return

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visornic/visornic_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 3da849a..234fa21 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -377,8 +377,8 @@ visornic_serverdown(struct visornic_devdata *devdata,
 			__func__);
 		spin_unlock_irqrestore(&devdata->priv_lock, flags);
 		return -EINVAL;
-	} else
-		spin_unlock_irqrestore(&devdata->priv_lock, flags);
+	}
+	spin_unlock_irqrestore(&devdata->priv_lock, flags);
 	return 0;
 }
 
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 14/14] staging: unisys: remove unnecessary goto

[Benjamin Romer]

parser_param_start() had a goto Away, which went to nothing but
a return statement. Remove the goto, the CamelCased label, and
just return directly.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>

---
v2: the patch was rebased.
v3: the patch was rebased.
---
 drivers/staging/unisys/visorbus/visorchipset.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 6ad03a5..a79aa2d 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -461,7 +461,8 @@ parser_param_start(struct parser_context *ctx,
 	struct spar_controlvm_parameters_header *phdr = NULL;
 
 	if (!ctx)
-		goto Away;
+		return;
+
 	phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
 	switch (which_string) {
 	case PARSERSTRING_INITIATOR:
@@ -483,9 +484,6 @@ parser_param_start(struct parser_context *ctx,
 	default:
 		break;
 	}
-
-Away:
-	return;
 }
 
 static void parser_done(struct parser_context *ctx)
-- 
2.5.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 13/14] staging: unisys: fix else statement in visornic_main.c

[Sudip Mukherjee]

On Mon, Feb 08, 2016 at 10:41:52AM -0500, Benjamin Romer wrote:
> From: Erik Arfvidson <erik.arfvidson@unisys.com>
> 
> This patch fixes the following types of check patch warnings:
> else is not generally useful after a break or return
> 
> Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> 
> ---
> v2: the patch was rebased.
> v3: the patch was rebased.
> ---
>  drivers/staging/unisys/visornic/visornic_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
> index 3da849a..234fa21 100644
> --- a/drivers/staging/unisys/visornic/visornic_main.c
> +++ b/drivers/staging/unisys/visornic/visornic_main.c
> @@ -377,8 +377,8 @@ visornic_serverdown(struct visornic_devdata *devdata,
>  			__func__);
>  		spin_unlock_irqrestore(&devdata->priv_lock, flags);
>  		return -EINVAL;
> -	} else
> -		spin_unlock_irqrestore(&devdata->priv_lock, flags);
> +	}
> +	spin_unlock_irqrestore(&devdata->priv_lock, flags);
>  	return 0;
>  }

maybe this is better where you have single exit point and so only one
spin_unlock_irqrestore().

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 3da849a..5d76497 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -358,15 +358,16 @@ visornic_serverdown(struct visornic_devdata *devdata,
 		    visorbus_state_complete_func complete_func)
 {
 	unsigned long flags;
+	int ret = 0;
 
 	spin_lock_irqsave(&devdata->priv_lock, flags);
 	if (!devdata->server_down && !devdata->server_change_state) {
 		if (devdata->going_away) {
-			spin_unlock_irqrestore(&devdata->priv_lock, flags);
 			dev_dbg(&devdata->dev->device,
 				"%s aborting because device removal pending\n",
 				__func__);
-			return -ENODEV;
+			ret = -ENODEV;
+			goto exit_unlock;
 		}
 		devdata->server_change_state = true;
 		devdata->server_down_complete_func = complete_func;
@@ -375,10 +376,11 @@ visornic_serverdown(struct visornic_devdata *devdata,
 	} else if (devdata->server_change_state) {
 		dev_dbg(&devdata->dev->device, "%s changing state\n",
 			__func__);
-		spin_unlock_irqrestore(&devdata->priv_lock, flags);
-		return -EINVAL;
-	} else
-		spin_unlock_irqrestore(&devdata->priv_lock, flags);
+		ret = -EINVAL;
+	}
+
+exit_unlock:
+	spin_unlock_irqrestore(&devdata->priv_lock, flags);
 	return 0;
 }


regards
sudip 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 13/14] staging: unisys: fix else statement in visornic_main.c

[Ben Romer]

On Mon, 2016-02-08 at 22:39 +0530, Sudip Mukherjee wrote:
> maybe this is better where you have single exit point and so only one
> spin_unlock_irqrestore().


We discussed this before. I don't want to put any of the goto messes
back in because I don't think it shortens the code or makes it any
simpler.

[snip]
> +
> +exit_unlock:
> +	spin_unlock_irqrestore(&devdata->priv_lock, flags);
>  	return 0;

This should be returning ret. 

It's less likely that someone will accidentally overwrite or throw away
the status later, if the code just returns directly at the point of
error.

-- Ben
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 13/14] staging: unisys: fix else statement in visornic_main.c

[Sudip Mukherjee]

On Tue, Feb 09, 2016 at 10:16:12AM -0500, Ben Romer wrote:
> On Mon, 2016-02-08 at 22:39 +0530, Sudip Mukherjee wrote:
> > maybe this is better where you have single exit point and so only one
> > spin_unlock_irqrestore().
> 
> 
> We discussed this before. I don't want to put any of the goto messes
> back in because I don't think it shortens the code or makes it any
> simpler.

yes, now i remember. Sorry for suggesting the same thing again.
Documentation/CodingStyle says:
"The goto statement comes in handy when a function exits from multiple
locations and some common work such as cleanup has to be done.  If there
is no cleanup needed then just return directly."

IMHO, the patch I posted or this suggestion does not go against that.
But anyway, since you are opposed to it I will not send the patch again.

> 
> [snip]
> > +
> > +exit_unlock:
> > +	spin_unlock_irqrestore(&devdata->priv_lock, flags);
> >  	return 0;
> 
> This should be returning ret.

yes, ofcourse. my mistake.

regards
sudip