[PATCH 00/28] staging: unisys: fix visorbus & visorinput issues raised by tglx

[PATCH 00/28] staging: unisys: fix visorbus & visorinput issues raised by tglx

[David Kershner]

This patchset comprises the first 26 patches of the previously-submitted
patchset (but retracted): 

	[PATCH v4 00/29] Fixed issues raised by tglx, then move visorbus to
			 drivers/virt

then adds 2 patches to visorinput that:
	* fixes a device initialization race condition
	* converts a semaphore to a mutex

As described in the email NAKing the previously-submitted patchset,
the reason we are re-submitting this now is to make things a bit
cleaner by separating the fixes we need to make to the code in
staging from the patchset that actually moves the code out of staging.

The intent of this patchset is to fix all known outstanding
issues with code in drivers/staging/unisys/, so that subsequent
patchsets can move these drivers out of staging.

tglx: The following patchset fixes issues you raised during your
code review of visorbus on 5/18, and visorinput on 6/1. 

Converts visorbus to use a kernel timer for periodic device-specific
callbacks instead of a workqueue, making the implementation in
periodic_work.c and periodic_work.h no longer necessary.  These files
are then deleted.

The visordriver_callback_lock has been switched to a mutex.

Several module parameters and structures were removed that were no
longer being used.

Bryan Thompson (4):
  staging: unisys: visorbus: Make visordriver_callback_lock a mutex
  staging: unisys: visorbus: Remove unnecessary EXPORT_SYMBOL statements
  staging: unisys: visorbus: Remove unused functions
  staging: unisys: Remove reference to unused STANDALONE_CLIENT

David Binder (13):
  staging: unisys: visorbus: remove unused module parameters
  staging: unisys: visorbus: remove unused struct
  staging: unisys: visorbus: modify format string to match argument
  staging: unisys: visornic: Correct comment spelling mistake
  staging: unisys: include: Remove thread-related enum members
  staging: unisys: visorbus: fix commenting in vbusdevinfo.h
  staging: unisys: visorbus: fix commenting in visorbus_main.c
  staging: unisys: visorbus: fix visorchannel.c comments
  staging: unisys: visorbus: Rectify commenting in visorchipset.c
  staging: unisys: visorbus: Move visorbus-unique functions to private
    header
  staging: unisys: visorbus: rectify kerneldoc comment for struct
  staging: unisys: visorbus: Remove notifier-related code from visorbus
  staging: unisys: visorbus: Rename function to follow existing
    convention

David Kershner (1):
  staging: unisys: Move vbushelper.h to visorbus directory

Erik Arfvidson (2):
  staging: unisys: visorbus: remove return values for write_vbus
    functions
  staging: unisys: visorbus: check parahotplug_request_complete_result

Tim Sell (8):
  staging: unisys: visorbus: removed unused periodic_test_workqueue
  staging: unisys: visorinput: remove unnecessary locking
  staging: unisys: visorbus: use kernel timer instead of workqueue
  staging: unisys: visorbus: remove periodic_work.h/.c
  staging: unisys: visorbus: remove unused parameter from function
  staging: unisys: visorbus: fix visorbus_private.h comments
  staging: unisys: visorinput: ensure proper locking wrt creation & ints
  staging: unisys: visorinput: make lock_visor_dev a mutex

 drivers/staging/unisys/include/guestlinuxdebug.h   |   4 +-
 drivers/staging/unisys/include/periodic_work.h     |  40 --
 drivers/staging/unisys/include/visorbus.h          |  80 +--
 drivers/staging/unisys/visorbus/Makefile           |   1 -
 drivers/staging/unisys/visorbus/periodic_work.c    | 204 -------
 drivers/staging/unisys/visorbus/vbusdeviceinfo.h   |  65 ++-
 .../unisys/{include => visorbus}/vbushelper.h      |   0
 drivers/staging/unisys/visorbus/visorbus_main.c    | 615 +++++++++++----------
 drivers/staging/unisys/visorbus/visorbus_private.h |  73 ++-
 drivers/staging/unisys/visorbus/visorchannel.c     | 269 +++------
 drivers/staging/unisys/visorbus/visorchipset.c     | 434 ++++++++-------
 drivers/staging/unisys/visorinput/visorinput.c     | 141 +++--
 drivers/staging/unisys/visornic/visornic_main.c    |   2 +-
 13 files changed, 833 insertions(+), 1095 deletions(-)
 delete mode 100644 drivers/staging/unisys/include/periodic_work.h
 delete mode 100644 drivers/staging/unisys/visorbus/periodic_work.c
 rename drivers/staging/unisys/{include => visorbus}/vbushelper.h (100%)

-- 
1.9.1

[PATCH 01/28] staging: unisys: visorbus: remove return values for write_vbus functions

[David Kershner]

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

This patch removes the return values from the write_vbus_* channel
functions. Nobody was checking the return values and the value stored
into the vbus info is not critical to the functioning of the device.

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 33 ++++++++++++++-----------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index d32b898..2ed9628 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -866,9 +866,12 @@ get_vbus_header_info(struct visorchannel *chan,
 
 /* Write the contents of <info> to the struct
  * spar_vbus_channel_protocol.chp_info.
+ *
+ * Returns void since this is debug information and not needed for
+ * device functionality.
  */
 
-static int
+static void
 write_vbus_chp_info(struct visorchannel *chan,
 		    struct spar_vbus_headerinfo *hdr_info,
 		    struct ultra_vbus_deviceinfo *info)
@@ -876,18 +879,19 @@ write_vbus_chp_info(struct visorchannel *chan,
 	int off = sizeof(struct channel_header) + hdr_info->chp_info_offset;
 
 	if (hdr_info->chp_info_offset == 0)
-		return -EFAULT;
+		return;
 
-	if (visorchannel_write(chan, off, info, sizeof(*info)) < 0)
-		return -EFAULT;
-	return 0;
+	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
 /* Write the contents of <info> to the struct
  * spar_vbus_channel_protocol.bus_info.
+ *
+ * Returns void since this is debug information and not needed for
+ * device functionality.
  */
 
-static int
+static void
 write_vbus_bus_info(struct visorchannel *chan,
 		    struct spar_vbus_headerinfo *hdr_info,
 		    struct ultra_vbus_deviceinfo *info)
@@ -895,17 +899,18 @@ write_vbus_bus_info(struct visorchannel *chan,
 	int off = sizeof(struct channel_header) + hdr_info->bus_info_offset;
 
 	if (hdr_info->bus_info_offset == 0)
-		return -EFAULT;
+		return;
 
-	if (visorchannel_write(chan, off, info, sizeof(*info)) < 0)
-		return -EFAULT;
-	return 0;
+	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
 /* Write the contents of <info> to the
  * struct spar_vbus_channel_protocol.dev_info[<devix>].
+ *
+ * Returns void since this is debug information and not needed for
+ * device functionality.
  */
-static int
+static void
 write_vbus_dev_info(struct visorchannel *chan,
 		    struct spar_vbus_headerinfo *hdr_info,
 		    struct ultra_vbus_deviceinfo *info, int devix)
@@ -915,11 +920,9 @@ write_vbus_dev_info(struct visorchannel *chan,
 	    (hdr_info->device_info_struct_bytes * devix);
 
 	if (hdr_info->dev_info_offset == 0)
-		return -EFAULT;
+		return;
 
-	if (visorchannel_write(chan, off, info, sizeof(*info)) < 0)
-		return -EFAULT;
-	return 0;
+	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
 /* For a child device just created on a client bus, fill in
-- 
1.9.1

[PATCH 02/28] staging: unisys: visorbus: check parahotplug_request_complete_result

[David Kershner]

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

This patch modifies the caller of parahotplug_request_complete()
to check the return value and return appropriate result.

Signed-off-by: Erik Arfvidson <erik.arfvidson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorchipset.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index d248c94..7f44913 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -2036,11 +2036,14 @@ static ssize_t devicedisabled_store(struct device *dev,
 				    const char *buf, size_t count)
 {
 	unsigned int id;
+	int err;
 
 	if (kstrtouint(buf, 10, &id))
 		return -EINVAL;
 
-	parahotplug_request_complete(id, 0);
+	err = parahotplug_request_complete(id, 0);
+	if (err < 0)
+		return err;
 	return count;
 }
 
-- 
1.9.1

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

[PATCH 03/28] staging: unisys: visorbus: remove unused module parameters

[David Kershner]

From: David Binder <david.binder@unisys.com>

Removes unused module parameters from visorbus_main.c, in response to
findings by SonarQube.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 2ed9628..71bff07 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -27,10 +27,9 @@
 #define MYDRVNAME "visorbus"
 
 /* module parameters */
-static int visorbus_debug;
 static int visorbus_forcematch;
 static int visorbus_forcenomatch;
-static int visorbus_debugref;
+
 #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024)
 
 /* Display string that is guaranteed to be no longer the 99 characters*/
@@ -1332,9 +1331,6 @@ visorbus_exit(void)
 	remove_bus_type();
 }
 
-module_param_named(debug, visorbus_debug, int, S_IRUGO);
-MODULE_PARM_DESC(visorbus_debug, "1 to debug");
-
 module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO);
 MODULE_PARM_DESC(visorbus_forcematch,
 		 "1 to force a successful dev <--> drv match");
@@ -1342,6 +1338,3 @@ MODULE_PARM_DESC(visorbus_forcematch,
 module_param_named(forcenomatch, visorbus_forcenomatch, int, S_IRUGO);
 MODULE_PARM_DESC(visorbus_forcenomatch,
 		 "1 to force an UNsuccessful dev <--> drv match");
-
-module_param_named(debugref, visorbus_debugref, int, S_IRUGO);
-MODULE_PARM_DESC(visorbus_debugref, "1 to debug reference counting");
-- 
1.9.1

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

[PATCH 04/28] staging: unisys: visorbus: remove unused struct

[David Kershner]

From: David Binder <david.binder@unisys.com>

Removes unused struct definition, channel_size_info, in response to
findings by SonarQube.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 71bff07..08d560c 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -1260,12 +1260,6 @@ chipset_device_resume(struct visor_device *dev_info)
 	initiate_chipset_device_pause_resume(dev_info, false);
 }
 
-struct channel_size_info {
-	uuid_le guid;
-	unsigned long min_size;
-	unsigned long max_size;
-};
-
 int
 visorbus_init(void)
 {
-- 
1.9.1

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

[PATCH 05/28] staging: unisys: visorbus: modify format string to match argument

[David Kershner]

From: David Binder <david.binder@unisys.com>

Modifies the format string of snprintf to expect an unsigned int
instead of a signed one, per the supplied argument.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 08d560c..756df41 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -433,7 +433,7 @@ static ssize_t client_bus_info_show(struct device *dev,
 		if (vdev->name)
 			partition_name = vdev->name;
 		shift = snprintf(pos, remain,
-				 "Client device / client driver info for %s eartition (vbus #%d):\n",
+				 "Client device / client driver info for %s eartition (vbus #%u):\n",
 				 partition_name, vdev->chipset_dev_no);
 		pos += shift;
 		remain -= shift;
-- 
1.9.1

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

[PATCH 06/28] staging: unisys: visornic: Correct comment spelling mistake

[David Kershner]

From: David Binder <david.binder@unisys.com>

Fixes a comment spelling mistake in visornic.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index a28388d..4fbe703 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -1586,7 +1586,7 @@ drain_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata)
  *
  *	Drain the respones queue of any responses from the IO partition.
  *	Process the responses as we get them.
- *	Returns when response queue is empty or when the threadd stops.
+ *	Returns when response queue is empty or when the thread stops.
  */
 static void
 service_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata,
-- 
1.9.1

[PATCH 07/28] staging: unisys: include: Remove thread-related enum members

[David Kershner]

From: David Binder <david.binder@unisys.com>

Code relating to ktheads was previously removed from s-Par driver code.
This patch cleans up lingering remnants of kthreads by removing thread-
related enum types.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/include/guestlinuxdebug.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/include/guestlinuxdebug.h b/drivers/staging/unisys/include/guestlinuxdebug.h
index b81287f..5af3f77 100644
--- a/drivers/staging/unisys/include/guestlinuxdebug.h
+++ b/drivers/staging/unisys/include/guestlinuxdebug.h
@@ -56,7 +56,7 @@ enum driver_pc {		/* POSTCODE driver identifier tuples */
 	UISLIB_PC = 0xD0,
 	UISLIB_PC_uislib_c = 0xD1,
 	UISLIB_PC_uisqueue_c = 0xD2,
-	UISLIB_PC_uisthread_c = 0xD3,
+	/* 0xD3 RESERVED */
 	UISLIB_PC_uisutils_c = 0xD4,
 };
 
@@ -91,7 +91,7 @@ enum event_pc {			/* POSTCODE event identifier tuples */
 	DRIVER_EXIT_PC = 0x0AC,
 	MALLOC_FAILURE_PC = 0x0AD,
 	QUEUE_DELAYED_WORK_PC = 0x0AE,
-	UISLIB_THREAD_FAILURE_PC = 0x0B7,
+	/* 0x0B7 RESERVED */
 	VBUS_CHANNEL_ENTRY_PC = 0x0B8,
 	VBUS_CHANNEL_FAILURE_PC = 0x0B9,
 	VBUS_CHANNEL_EXIT_PC = 0x0BA,
-- 
1.9.1

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

[PATCH 08/28] staging: unisys: visorbus: removed unused periodic_test_workqueue

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

periodic_test_workqueue was an unused relic from the past, and was removed.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 756df41..ebdd5de 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -116,14 +116,6 @@ struct bus_type visorbus_type = {
 	.bus_groups = visorbus_bus_groups,
 };
 
-static struct delayed_work periodic_work;
-
-/* YES, we need 2 workqueues.
- * The reason is, workitems on the test queue may need to cancel
- * workitems on the other queue.  You will be in for trouble if you try to
- * do this with workitems queued on the same workqueue.
- */
-static struct workqueue_struct *periodic_test_workqueue;
 static struct workqueue_struct *periodic_dev_workqueue;
 static long long bus_count;	/** number of bus instances */
 					/** ever-increasing */
@@ -1309,13 +1301,6 @@ visorbus_exit(void)
 	destroy_workqueue(periodic_dev_workqueue);
 	periodic_dev_workqueue = NULL;
 
-	if (periodic_test_workqueue) {
-		cancel_delayed_work(&periodic_work);
-		flush_workqueue(periodic_test_workqueue);
-		destroy_workqueue(periodic_test_workqueue);
-		periodic_test_workqueue = NULL;
-	}
-
 	list_for_each_safe(listentry, listtmp, &list_all_bus_instances) {
 		struct visor_device *dev = list_entry(listentry,
 						      struct visor_device,
-- 
1.9.1

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

[PATCH 09/28] staging: unisys: visorinput: remove unnecessary locking

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

Locking in the _interrupt() function is NOT necessary so long as we ensure
that interrupts have been stopped whenever we need to pause or resume the
device, which we now do.

While a device is paused, we ensure that interrupts stay disabled, i.e.
that the _interrupt() function will NOT be called, yet remember the desired
state in devdata->interrupts_enabled if open() or close() are called are
called while the device is paused.  Then when the device is resumed, we
restore the actual state of interrupts (i.e., whether _interrupt() is going
to be called or not) to the desired state in devdata->interrupts_enabled.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorinput/visorinput.c | 57 +++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index d67cd763..f633985 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -66,6 +66,7 @@ struct visorinput_devdata {
 	struct rw_semaphore lock_visor_dev; /* lock for dev */
 	struct input_dev *visorinput_dev;
 	bool paused;
+	bool interrupts_enabled;
 	unsigned int keycode_table_bytes; /* size of following array */
 	/* for keyboard devices: visorkbd_keycode[] + visorkbd_ext_keycode[] */
 	unsigned char keycode_table[0];
@@ -228,7 +229,21 @@ static int visorinput_open(struct input_dev *visorinput_dev)
 		return -EINVAL;
 	}
 	dev_dbg(&visorinput_dev->dev, "%s opened\n", __func__);
+
+	/*
+	 * If we're not paused, really enable interrupts.
+	 * Regardless of whether we are paused, set a flag indicating
+	 * interrupts should be enabled so when we resume, interrupts
+	 * will really be enabled.
+	 */
+	down_write(&devdata->lock_visor_dev);
+	devdata->interrupts_enabled = true;
+	if (devdata->paused)
+		goto out_unlock;
 	visorbus_enable_channel_interrupts(devdata->dev);
+
+out_unlock:
+	up_write(&devdata->lock_visor_dev);
 	return 0;
 }
 
@@ -243,7 +258,22 @@ static void visorinput_close(struct input_dev *visorinput_dev)
 		return;
 	}
 	dev_dbg(&visorinput_dev->dev, "%s closed\n", __func__);
+
+	/*
+	 * If we're not paused, really disable interrupts.
+	 * Regardless of whether we are paused, set a flag indicating
+	 * interrupts should be disabled so when we resume we will
+	 * not re-enable them.
+	 */
+
+	down_write(&devdata->lock_visor_dev);
+	devdata->interrupts_enabled = false;
+	if (devdata->paused)
+		goto out_unlock;
 	visorbus_disable_channel_interrupts(devdata->dev);
+
+out_unlock:
+	up_write(&devdata->lock_visor_dev);
 }
 
 /*
@@ -438,10 +468,8 @@ visorinput_remove(struct visor_device *dev)
 	 * in visorinput_channel_interrupt()
 	 */
 
-	down_write(&devdata->lock_visor_dev);
 	dev_set_drvdata(&dev->device, NULL);
 	unregister_client_input(devdata->visorinput_dev);
-	up_write(&devdata->lock_visor_dev);
 	kfree(devdata);
 }
 
@@ -529,13 +557,7 @@ visorinput_channel_interrupt(struct visor_device *dev)
 	if (!devdata)
 		return;
 
-	down_write(&devdata->lock_visor_dev);
-	if (devdata->paused) /* don't touch device/channel when paused */
-		goto out_locked;
-
 	visorinput_dev = devdata->visorinput_dev;
-	if (!visorinput_dev)
-		goto out_locked;
 
 	while (visorchannel_signalremove(dev->visorchannel, 0, &r)) {
 		scancode = r.activity.arg1;
@@ -611,8 +633,6 @@ visorinput_channel_interrupt(struct visor_device *dev)
 			break;
 		}
 	}
-out_locked:
-	up_write(&devdata->lock_visor_dev);
 }
 
 static int
@@ -632,6 +652,14 @@ visorinput_pause(struct visor_device *dev,
 		rc = -EBUSY;
 		goto out_locked;
 	}
+	if (devdata->interrupts_enabled)
+		visorbus_disable_channel_interrupts(dev);
+
+	/*
+	 * due to above, at this time no thread of execution will be
+	 * in visorinput_channel_interrupt()
+	 */
+
 	devdata->paused = true;
 	complete_func(dev, 0);
 	rc = 0;
@@ -659,6 +687,15 @@ visorinput_resume(struct visor_device *dev,
 	}
 	devdata->paused = false;
 	complete_func(dev, 0);
+
+	/*
+	 * Re-establish calls to visorinput_channel_interrupt() if that is
+	 * the desired state that we've kept track of in interrupts_enabled
+	 * while the device was paused.
+	 */
+	if (devdata->interrupts_enabled)
+		visorbus_enable_channel_interrupts(dev);
+
 	rc = 0;
 out_locked:
 	up_write(&devdata->lock_visor_dev);
-- 
1.9.1

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

[PATCH 10/28] staging: unisys: visorbus: use kernel timer instead of workqueue

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

A kernel timer is now used as the vehicle to periodically call the
channel_interrupt function of registered visor drivers, instead of a
workqueue.

This simplifies a lot of things by making periodic_work.c and
periodic_work.h no longer necessary.  This change also means that the
channel_interrupt() callbacks registered by visor drivers (via
visorbus_register_visor_driver()) will now be called in atomic context
(i.e., canNOT sleep) rather than kernel thread context (CAN sleep).
Fortunately this did NOT necessitate any change to the existing
channel_interrupt() callbacks, because none of them ever perform any
operations that would be invalid in atomic context.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       | 10 +++--
 drivers/staging/unisys/visorbus/visorbus_main.c | 54 +++++++------------------
 2 files changed, 21 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 9baf1ec..9bb88bb 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -34,8 +34,9 @@
 #include <linux/poll.h>
 #include <linux/kernel.h>
 #include <linux/uuid.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
 
-#include "periodic_work.h"
 #include "channel.h"
 
 struct visor_driver;
@@ -126,8 +127,8 @@ struct visor_driver {
  * device:			Device struct meant for use by the bus driver
  *				only.
  * list_all:			Used by the bus driver to enumerate devices.
- * periodic_work:		Device work queue. Private use by bus driver
- *				only.
+ * timer:		        Timer fired periodically to do interrupt-type
+ *				activity.
  * being_removed:		Indicates that the device is being removed from
  *				the bus. Private bus driver use only.
  * visordriver_callback_lock:	Used by the bus driver to lock when handling
@@ -157,7 +158,8 @@ struct visor_device {
 	/* These fields are for private use by the bus driver only. */
 	struct device device;
 	struct list_head list_all;
-	struct periodic_work *periodic_work;
+	struct timer_list timer;
+	bool timer_active;
 	bool being_removed;
 	struct semaphore visordriver_callback_lock;
 	bool pausing;
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index ebdd5de..e98e720 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -19,7 +19,6 @@
 #include "visorbus.h"
 #include "visorbus_private.h"
 #include "version.h"
-#include "periodic_work.h"
 #include "vbuschannel.h"
 #include "guestlinuxdebug.h"
 #include "vmcallinterface.h"
@@ -116,7 +115,6 @@ struct bus_type visorbus_type = {
 	.bus_groups = visorbus_bus_groups,
 };
 
-static struct workqueue_struct *periodic_dev_workqueue;
 static long long bus_count;	/** number of bus instances */
 					/** ever-increasing */
 
@@ -222,10 +220,6 @@ visorbus_release_device(struct device *xdev)
 {
 	struct visor_device *dev = to_visor_device(xdev);
 
-	if (dev->periodic_work) {
-		visor_periodic_work_destroy(dev->periodic_work);
-		dev->periodic_work = NULL;
-	}
 	if (dev->visorchannel) {
 		visorchannel_destroy(dev->visorchannel);
 		dev->visorchannel = NULL;
@@ -530,35 +524,36 @@ unregister_driver_attributes(struct visor_driver *drv)
 }
 
 static void
-dev_periodic_work(void *xdev)
+dev_periodic_work(unsigned long __opaque)
 {
-	struct visor_device *dev = xdev;
+	struct visor_device *dev = (struct visor_device *)__opaque;
 	struct visor_driver *drv = to_visor_driver(dev->device.driver);
 
-	down(&dev->visordriver_callback_lock);
 	if (drv->channel_interrupt)
 		drv->channel_interrupt(dev);
-	up(&dev->visordriver_callback_lock);
-	if (!visor_periodic_work_nextperiod(dev->periodic_work))
-		put_device(&dev->device);
+	mod_timer(&dev->timer, jiffies + POLLJIFFIES_NORMALCHANNEL);
 }
 
 static void
 dev_start_periodic_work(struct visor_device *dev)
 {
-	if (dev->being_removed)
+	if (dev->being_removed || dev->timer_active)
 		return;
 	/* now up by at least 2 */
 	get_device(&dev->device);
-	if (!visor_periodic_work_start(dev->periodic_work))
-		put_device(&dev->device);
+	dev->timer.expires = jiffies + POLLJIFFIES_NORMALCHANNEL;
+	add_timer(&dev->timer);
+	dev->timer_active = true;
 }
 
 static void
 dev_stop_periodic_work(struct visor_device *dev)
 {
-	if (visor_periodic_work_stop(dev->periodic_work))
-		put_device(&dev->device);
+	if (!dev->timer_active)
+		return;
+	del_timer_sync(&dev->timer);
+	dev->timer_active = false;
+	put_device(&dev->device);
 }
 
 /** This is called automatically upon adding a visor_device (device_add), or
@@ -776,17 +771,9 @@ create_visor_device(struct visor_device *dev)
 	dev->device.release = visorbus_release_device;
 	/* keep a reference just for us (now 2) */
 	get_device(&dev->device);
-	dev->periodic_work =
-		visor_periodic_work_create(POLLJIFFIES_NORMALCHANNEL,
-					   periodic_dev_workqueue,
-					   dev_periodic_work,
-					   dev, dev_name(&dev->device));
-	if (!dev->periodic_work) {
-		POSTCODE_LINUX_3(DEVICE_CREATE_FAILURE_PC, chipset_dev_no,
-				 DIAG_SEVERITY_ERR);
-		err = -EINVAL;
-		goto err_put;
-	}
+	init_timer(&dev->timer);
+	dev->timer.data = (unsigned long)(dev);
+	dev->timer.function = dev_periodic_work;
 
 	/* bus_id must be a unique name with respect to this bus TYPE
 	 * (NOT bus instance).  That's why we need to include the bus
@@ -1268,13 +1255,6 @@ visorbus_init(void)
 		goto error;
 	}
 
-	periodic_dev_workqueue = create_singlethread_workqueue("visorbus_dev");
-	if (!periodic_dev_workqueue) {
-		POSTCODE_LINUX_2(CREATE_WORKQUEUE_PC, DIAG_SEVERITY_ERR);
-		err = -ENOMEM;
-		goto error;
-	}
-
 	/* This enables us to receive notifications when devices appear for
 	 * which this service partition is to be a server for.
 	 */
@@ -1297,10 +1277,6 @@ visorbus_exit(void)
 	visorchipset_register_busdev(NULL, NULL, NULL);
 	remove_all_visor_devices();
 
-	flush_workqueue(periodic_dev_workqueue); /* better not be any work! */
-	destroy_workqueue(periodic_dev_workqueue);
-	periodic_dev_workqueue = NULL;
-
 	list_for_each_safe(listentry, listtmp, &list_all_bus_instances) {
 		struct visor_device *dev = list_entry(listentry,
 						      struct visor_device,
-- 
1.9.1

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

[PATCH 11/28] staging: unisys: visorbus: remove periodic_work.h/.c

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

These files were made no-longer-necessary by recent commits.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/include/periodic_work.h  |  40 -----
 drivers/staging/unisys/visorbus/Makefile        |   1 -
 drivers/staging/unisys/visorbus/periodic_work.c | 204 ------------------------
 drivers/staging/unisys/visorbus/visorchipset.c  |   1 -
 4 files changed, 246 deletions(-)
 delete mode 100644 drivers/staging/unisys/include/periodic_work.h
 delete mode 100644 drivers/staging/unisys/visorbus/periodic_work.c

diff --git a/drivers/staging/unisys/include/periodic_work.h b/drivers/staging/unisys/include/periodic_work.h
deleted file mode 100644
index 0b3335a..0000000
--- a/drivers/staging/unisys/include/periodic_work.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/* periodic_work.h
- *
- * Copyright (C) 2010 - 2013 UNISYS CORPORATION
- * All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT.  See the GNU General Public License for more
- * details.
- */
-
-#ifndef __PERIODIC_WORK_H__
-#define __PERIODIC_WORK_H__
-
-#include <linux/seq_file.h>
-#include <linux/slab.h>
-
-/* PERIODIC_WORK an opaque structure to users.
- * Fields are declared only in the implementation .c files.
- */
-struct periodic_work;
-
-struct periodic_work *
-visor_periodic_work_create(ulong jiffy_interval,
-			   struct workqueue_struct *workqueue,
-			   void (*workfunc)(void *),
-			   void *workfuncarg,
-			   const char *devnam);
-void visor_periodic_work_destroy(struct periodic_work *pw);
-bool visor_periodic_work_nextperiod(struct periodic_work *pw);
-bool visor_periodic_work_start(struct periodic_work *pw);
-bool visor_periodic_work_stop(struct periodic_work *pw);
-
-#endif
diff --git a/drivers/staging/unisys/visorbus/Makefile b/drivers/staging/unisys/visorbus/Makefile
index fc790e7..f3730d8 100644
--- a/drivers/staging/unisys/visorbus/Makefile
+++ b/drivers/staging/unisys/visorbus/Makefile
@@ -7,6 +7,5 @@ obj-$(CONFIG_UNISYS_VISORBUS)	+= visorbus.o
 visorbus-y := visorbus_main.o
 visorbus-y += visorchannel.o
 visorbus-y += visorchipset.o
-visorbus-y += periodic_work.o
 
 ccflags-y += -Idrivers/staging/unisys/include
diff --git a/drivers/staging/unisys/visorbus/periodic_work.c b/drivers/staging/unisys/visorbus/periodic_work.c
deleted file mode 100644
index 00b1527..0000000
--- a/drivers/staging/unisys/visorbus/periodic_work.c
+++ /dev/null
@@ -1,204 +0,0 @@
-/* periodic_work.c
- *
- * Copyright (C) 2010 - 2015 UNISYS CORPORATION
- * All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT.  See the GNU General Public License for more
- * details.
- */
-
-/*
- *  Helper functions to schedule periodic work in Linux kernel mode.
- */
-#include <linux/sched.h>
-
-#include "periodic_work.h"
-
-#define MYDRVNAME "periodic_work"
-
-struct periodic_work {
-	rwlock_t lock;
-	struct delayed_work work;
-	void (*workfunc)(void *);
-	void *workfuncarg;
-	bool is_scheduled;
-	bool want_to_stop;
-	ulong jiffy_interval;
-	struct workqueue_struct *workqueue;
-	const char *devnam;
-};
-
-static void periodic_work_func(struct work_struct *work)
-{
-	struct periodic_work *pw;
-
-	pw = container_of(work, struct periodic_work, work.work);
-	(*pw->workfunc)(pw->workfuncarg);
-}
-
-struct periodic_work
-*visor_periodic_work_create(ulong jiffy_interval,
-			    struct workqueue_struct *workqueue,
-			    void (*workfunc)(void *),
-			    void *workfuncarg,
-			    const char *devnam)
-{
-	struct periodic_work *pw;
-
-	pw = kzalloc(sizeof(*pw), GFP_KERNEL | __GFP_NORETRY);
-	if (!pw)
-		return NULL;
-
-	rwlock_init(&pw->lock);
-	pw->jiffy_interval = jiffy_interval;
-	pw->workqueue = workqueue;
-	pw->workfunc = workfunc;
-	pw->workfuncarg = workfuncarg;
-	pw->devnam = devnam;
-	return pw;
-}
-EXPORT_SYMBOL_GPL(visor_periodic_work_create);
-
-void visor_periodic_work_destroy(struct periodic_work *pw)
-{
-	kfree(pw);
-}
-EXPORT_SYMBOL_GPL(visor_periodic_work_destroy);
-
-/** Call this from your periodic work worker function to schedule the next
- *  call.
- *  If this function returns false, there was a failure and the
- *  periodic work is no longer scheduled
- */
-bool visor_periodic_work_nextperiod(struct periodic_work *pw)
-{
-	bool rc = false;
-
-	write_lock(&pw->lock);
-	if (pw->want_to_stop) {
-		pw->is_scheduled = false;
-		pw->want_to_stop = false;
-		rc = true;  /* yes, true; see visor_periodic_work_stop() */
-		goto unlock;
-	} else if (!queue_delayed_work(pw->workqueue, &pw->work,
-				       pw->jiffy_interval)) {
-		pw->is_scheduled = false;
-		rc = false;
-		goto unlock;
-	}
-	rc = true;
-unlock:
-	write_unlock(&pw->lock);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(visor_periodic_work_nextperiod);
-
-/** This function returns true iff new periodic work was actually started.
- *  If this function returns false, then no work was started
- *  (either because it was already started, or because of a failure).
- */
-bool visor_periodic_work_start(struct periodic_work *pw)
-{
-	bool rc = false;
-
-	write_lock(&pw->lock);
-	if (pw->is_scheduled) {
-		rc = false;
-		goto unlock;
-	}
-	if (pw->want_to_stop) {
-		rc = false;
-		goto unlock;
-	}
-	INIT_DELAYED_WORK(&pw->work, &periodic_work_func);
-	if (!queue_delayed_work(pw->workqueue, &pw->work,
-				pw->jiffy_interval)) {
-		rc = false;
-		goto unlock;
-	}
-	pw->is_scheduled = true;
-	rc = true;
-unlock:
-	write_unlock(&pw->lock);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(visor_periodic_work_start);
-
-/** This function returns true iff your call actually stopped the periodic
- *  work.
- *
- *  -- PAY ATTENTION... this is important --
- *
- *  NO NO #1
- *
- *     Do NOT call this function from some function that is running on the
- *     same workqueue as the work you are trying to stop might be running
- *     on!  If you violate this rule, visor_periodic_work_stop() MIGHT work,
- *     but it also MIGHT get hung up in an infinite loop saying
- *     "waiting for delayed work...".  This will happen if the delayed work
- *     you are trying to cancel has been put in the workqueue list, but can't
- *     run yet because we are running that same workqueue thread right now.
- *
- *     Bottom line: If you need to call visor_periodic_work_stop() from a
- *     workitem, be sure the workitem is on a DIFFERENT workqueue than the
- *     workitem that you are trying to cancel.
- *
- *     If I could figure out some way to check for this "no no" condition in
- *     the code, I would.  It would have saved me the trouble of writing this
- *     long comment.  And also, don't think this is some "theoretical" race
- *     condition.  It is REAL, as I have spent the day chasing it.
- *
- *  NO NO #2
- *
- *     Take close note of the locks that you own when you call this function.
- *     You must NOT own any locks that are needed by the periodic work
- *     function that is currently installed.  If you DO, a deadlock may result,
- *     because stopping the periodic work often involves waiting for the last
- *     iteration of the periodic work function to complete.  Again, if you hit
- *     this deadlock, you will get hung up in an infinite loop saying
- *     "waiting for delayed work...".
- */
-bool visor_periodic_work_stop(struct periodic_work *pw)
-{
-	bool stopped_something = false;
-
-	write_lock(&pw->lock);
-	stopped_something = pw->is_scheduled && (!pw->want_to_stop);
-	while (pw->is_scheduled) {
-		pw->want_to_stop = true;
-		if (cancel_delayed_work(&pw->work)) {
-			/* We get here if the delayed work was pending as
-			 * delayed work, but was NOT run.
-			 */
-			WARN_ON(!pw->is_scheduled);
-			pw->is_scheduled = false;
-		} else {
-			/* If we get here, either the delayed work:
-			 * - was run, OR,
-			 * - is running RIGHT NOW on another processor, OR,
-			 * - wasn't even scheduled (there is a miniscule
-			 *   timing window where this could be the case)
-			 * flush_workqueue() would make sure it is finished
-			 * executing, but that still isn't very useful, which
-			 * explains the loop...
-			 */
-		}
-		if (pw->is_scheduled) {
-			write_unlock(&pw->lock);
-			schedule_timeout_interruptible(msecs_to_jiffies(10));
-			write_lock(&pw->lock);
-		} else {
-			pw->want_to_stop = false;
-		}
-	}
-	write_unlock(&pw->lock);
-	return stopped_something;
-}
-EXPORT_SYMBOL_GPL(visor_periodic_work_stop);
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 7f44913..b668b80 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -29,7 +29,6 @@
 #include "controlvmchannel.h"
 #include "controlvmcompletionstatus.h"
 #include "guestlinuxdebug.h"
-#include "periodic_work.h"
 #include "version.h"
 #include "visorbus.h"
 #include "visorbus_private.h"
-- 
1.9.1

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

[PATCH 12/28] staging: unisys: visorbus: Make visordriver_callback_lock a mutex

[David Kershner]

From: Bryan Thompson <bryan.thompson@unisys.com>

visordriver_callback_lock is just a binary semaphore that logically
makes more sense as a mutex.

Signed-off-by: Bryan Thompson <bryan.thompson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       |  2 +-
 drivers/staging/unisys/visorbus/visorbus_main.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 9bb88bb..3ba01cf 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -161,7 +161,7 @@ struct visor_device {
 	struct timer_list timer;
 	bool timer_active;
 	bool being_removed;
-	struct semaphore visordriver_callback_lock;
+	struct mutex visordriver_callback_lock;
 	bool pausing;
 	bool resuming;
 	u32 chipset_bus_no;
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index e98e720..2e0eb03 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -574,7 +574,7 @@ visordriver_probe_device(struct device *xdev)
 	if (!drv->probe)
 		return -ENODEV;
 
-	down(&dev->visordriver_callback_lock);
+	mutex_lock(&dev->visordriver_callback_lock);
 	dev->being_removed = false;
 
 	res = drv->probe(dev);
@@ -584,7 +584,7 @@ visordriver_probe_device(struct device *xdev)
 		fix_vbus_dev_info(dev);
 	}
 
-	up(&dev->visordriver_callback_lock);
+	mutex_unlock(&dev->visordriver_callback_lock);
 	return res;
 }
 
@@ -600,11 +600,11 @@ visordriver_remove_device(struct device *xdev)
 
 	dev = to_visor_device(xdev);
 	drv = to_visor_driver(xdev->driver);
-	down(&dev->visordriver_callback_lock);
+	mutex_lock(&dev->visordriver_callback_lock);
 	dev->being_removed = true;
 	if (drv->remove)
 		drv->remove(dev);
-	up(&dev->visordriver_callback_lock);
+	mutex_unlock(&dev->visordriver_callback_lock);
 	dev_stop_periodic_work(dev);
 
 	put_device(&dev->device);
@@ -764,7 +764,7 @@ create_visor_device(struct visor_device *dev)
 	POSTCODE_LINUX_4(DEVICE_CREATE_ENTRY_PC, chipset_dev_no, chipset_bus_no,
 			 POSTCODE_SEVERITY_INFO);
 
-	sema_init(&dev->visordriver_callback_lock, 1);	/* unlocked */
+	mutex_init(&dev->visordriver_callback_lock);
 	dev->device.bus = &visorbus_type;
 	dev->device.groups = visorbus_channel_groups;
 	device_initialize(&dev->device);
-- 
1.9.1

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

[PATCH 13/28] staging: unisys: visorbus: Remove unnecessary EXPORT_SYMBOL statements

[David Kershner]

From: Bryan Thompson <bryan.thompson@unisys.com>

The driver that is now visorbus started out as multiple separate drivers,
and when they were merged the EXPORT_SYMBOL statements that were required
for separate drivers were left in the code. This patch removes those now
unnecessary exports.

Signed-off-by: Bryan Thompson <bryan.thompson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c |  1 -
 drivers/staging/unisys/visorbus/visorchannel.c  | 17 -----------------
 drivers/staging/unisys/visorbus/visorchipset.c  |  2 --
 3 files changed, 20 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 2e0eb03..f7f5986 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -721,7 +721,6 @@ visorbus_clear_channel(struct visor_device *dev, unsigned long offset, u8 ch,
 {
 	return visorchannel_clear(dev->visorchannel, offset, ch, nbytes);
 }
-EXPORT_SYMBOL_GPL(visorbus_clear_channel);
 
 /** We don't really have a real interrupt, so for now we just call the
  *  interrupt function periodically...
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 4337358..1f626c3 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -148,7 +148,6 @@ visorchannel_create(u64 physaddr, unsigned long channel_bytes,
 	return visorchannel_create_guts(physaddr, channel_bytes, gfp, 0, guid,
 					false);
 }
-EXPORT_SYMBOL_GPL(visorchannel_create);
 
 struct visorchannel *
 visorchannel_create_with_lock(u64 physaddr, unsigned long channel_bytes,
@@ -157,7 +156,6 @@ visorchannel_create_with_lock(u64 physaddr, unsigned long channel_bytes,
 	return visorchannel_create_guts(physaddr, channel_bytes, gfp, 0, guid,
 					true);
 }
-EXPORT_SYMBOL_GPL(visorchannel_create_with_lock);
 
 void
 visorchannel_destroy(struct visorchannel *channel)
@@ -171,21 +169,18 @@ visorchannel_destroy(struct visorchannel *channel)
 	}
 	kfree(channel);
 }
-EXPORT_SYMBOL_GPL(visorchannel_destroy);
 
 u64
 visorchannel_get_physaddr(struct visorchannel *channel)
 {
 	return channel->physaddr;
 }
-EXPORT_SYMBOL_GPL(visorchannel_get_physaddr);
 
 ulong
 visorchannel_get_nbytes(struct visorchannel *channel)
 {
 	return channel->nbytes;
 }
-EXPORT_SYMBOL_GPL(visorchannel_get_nbytes);
 
 char *
 visorchannel_uuid_id(uuid_le *guid, char *s)
@@ -193,28 +188,24 @@ visorchannel_uuid_id(uuid_le *guid, char *s)
 	sprintf(s, "%pUL", guid);
 	return s;
 }
-EXPORT_SYMBOL_GPL(visorchannel_uuid_id);
 
 char *
 visorchannel_id(struct visorchannel *channel, char *s)
 {
 	return visorchannel_uuid_id(&channel->guid, s);
 }
-EXPORT_SYMBOL_GPL(visorchannel_id);
 
 char *
 visorchannel_zoneid(struct visorchannel *channel, char *s)
 {
 	return visorchannel_uuid_id(&channel->chan_hdr.zone_uuid, s);
 }
-EXPORT_SYMBOL_GPL(visorchannel_zoneid);
 
 u64
 visorchannel_get_clientpartition(struct visorchannel *channel)
 {
 	return channel->chan_hdr.partition_handle;
 }
-EXPORT_SYMBOL_GPL(visorchannel_get_clientpartition);
 
 int
 visorchannel_set_clientpartition(struct visorchannel *channel,
@@ -223,7 +214,6 @@ visorchannel_set_clientpartition(struct visorchannel *channel,
 	channel->chan_hdr.partition_handle = partition_handle;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(visorchannel_set_clientpartition);
 
 uuid_le
 visorchannel_get_uuid(struct visorchannel *channel)
@@ -243,7 +233,6 @@ visorchannel_read(struct visorchannel *channel, ulong offset,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(visorchannel_read);
 
 int
 visorchannel_write(struct visorchannel *channel, ulong offset,
@@ -265,7 +254,6 @@ visorchannel_write(struct visorchannel *channel, ulong offset,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(visorchannel_write);
 
 int
 visorchannel_clear(struct visorchannel *channel, ulong offset, u8 ch,
@@ -301,14 +289,12 @@ out_free_page:
 	free_page((unsigned long)buf);
 	return err;
 }
-EXPORT_SYMBOL_GPL(visorchannel_clear);
 
 void __iomem  *
 visorchannel_get_header(struct visorchannel *channel)
 {
 	return (void __iomem *)&channel->chan_hdr;
 }
-EXPORT_SYMBOL_GPL(visorchannel_get_header);
 
 /** Return offset of a specific SIGNAL_QUEUE_HEADER from the beginning of a
  *  channel header
@@ -522,7 +508,6 @@ visorchannel_signalqueue_slots_avail(struct visorchannel *channel, u32 queue)
 	slots_avail = sig_hdr.max_signals - slots_used;
 	return (int)slots_avail;
 }
-EXPORT_SYMBOL_GPL(visorchannel_signalqueue_slots_avail);
 
 int
 visorchannel_signalqueue_max_slots(struct visorchannel *channel, u32 queue)
@@ -533,7 +518,6 @@ visorchannel_signalqueue_max_slots(struct visorchannel *channel, u32 queue)
 		return 0;
 	return (int)sig_hdr.max_signals;
 }
-EXPORT_SYMBOL_GPL(visorchannel_signalqueue_max_slots);
 
 static void
 sigqueue_debug(struct signal_queue_header *q, int which, struct seq_file *seq)
@@ -632,4 +616,3 @@ visorchannel_debug(struct visorchannel *channel, int num_queues,
 	seq_printf(seq, "--- End   channel @0x%-16.16Lx for 0x%lx bytes ---\n",
 		   addr + off, nbytes);
 }
-EXPORT_SYMBOL_GPL(visorchannel_debug);
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index b668b80..b5942c6 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -682,7 +682,6 @@ struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
 		vdev = to_visor_device(dev);
 	return vdev;
 }
-EXPORT_SYMBOL(visorbus_get_device_by_id);
 
 void
 visorchipset_register_busdev(
@@ -707,7 +706,6 @@ visorchipset_register_busdev(
 
 	up(&notifier_lock);
 }
-EXPORT_SYMBOL_GPL(visorchipset_register_busdev);
 
 static void
 chipset_init(struct controlvm_message *inmsg)
-- 
1.9.1

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

[PATCH 14/28] staging: unisys: visorbus: Remove unused functions

[David Kershner]

From: Bryan Thompson <bryan.thompson@unisys.com>

Remove visorbus_clear_channel, visorchannel_signalqueue_slots_avail,
visorchannel_signalqueue_max_slots, visorchannel_clear, and
visorchannel_debug which are no longer called by any driver.

Signed-off-by: Bryan Thompson <bryan.thompson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       |   9 --
 drivers/staging/unisys/visorbus/visorbus_main.c |   7 --
 drivers/staging/unisys/visorbus/visorchannel.c  | 161 ------------------------
 3 files changed, 177 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 3ba01cf..cbe240a 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -185,8 +185,6 @@ int visorbus_read_channel(struct visor_device *dev,
 int visorbus_write_channel(struct visor_device *dev,
 			   unsigned long offset, void *src,
 			   unsigned long nbytes);
-int visorbus_clear_channel(struct visor_device *dev,
-			   unsigned long offset, u8 ch, unsigned long nbytes);
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
 #endif
@@ -206,17 +204,12 @@ int visorchannel_read(struct visorchannel *channel, ulong offset,
 		      void *local, ulong nbytes);
 int visorchannel_write(struct visorchannel *channel, ulong offset,
 		       void *local, ulong nbytes);
-int visorchannel_clear(struct visorchannel *channel, ulong offset,
-		       u8 ch, ulong nbytes);
 bool visorchannel_signalremove(struct visorchannel *channel, u32 queue,
 			       void *msg);
 bool visorchannel_signalinsert(struct visorchannel *channel, u32 queue,
 			       void *msg);
 bool visorchannel_signalempty(struct visorchannel *channel, u32 queue);
 
-int visorchannel_signalqueue_slots_avail(struct visorchannel *channel,
-					 u32 queue);
-int visorchannel_signalqueue_max_slots(struct visorchannel *channel, u32 queue);
 u64 visorchannel_get_physaddr(struct visorchannel *channel);
 ulong visorchannel_get_nbytes(struct visorchannel *channel);
 char *visorchannel_id(struct visorchannel *channel, char *s);
@@ -226,8 +219,6 @@ int visorchannel_set_clientpartition(struct visorchannel *channel,
 				     u64 partition_handle);
 uuid_le visorchannel_get_uuid(struct visorchannel *channel);
 char *visorchannel_uuid_id(uuid_le *guid, char *s);
-void visorchannel_debug(struct visorchannel *channel, int num_queues,
-			struct seq_file *seq, u32 off);
 void __iomem *visorchannel_get_header(struct visorchannel *channel);
 
 #define BUS_ROOT_DEVICE		UINT_MAX
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index f7f5986..8c1754f9 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -715,13 +715,6 @@ visorbus_write_channel(struct visor_device *dev, unsigned long offset,
 }
 EXPORT_SYMBOL_GPL(visorbus_write_channel);
 
-int
-visorbus_clear_channel(struct visor_device *dev, unsigned long offset, u8 ch,
-		       unsigned long nbytes)
-{
-	return visorchannel_clear(dev->visorchannel, offset, ch, nbytes);
-}
-
 /** We don't really have a real interrupt, so for now we just call the
  *  interrupt function periodically...
  */
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 1f626c3..43315c2 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -255,41 +255,6 @@ visorchannel_write(struct visorchannel *channel, ulong offset,
 	return 0;
 }
 
-int
-visorchannel_clear(struct visorchannel *channel, ulong offset, u8 ch,
-		   ulong nbytes)
-{
-	int err;
-	int bufsize = PAGE_SIZE;
-	int written = 0;
-	u8 *buf;
-
-	buf = (u8 *)__get_free_page(GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	memset(buf, ch, bufsize);
-
-	while (nbytes > 0) {
-		int thisbytes = bufsize;
-
-		if (nbytes < thisbytes)
-			thisbytes = nbytes;
-		err = visorchannel_write(channel, offset + written,
-					 buf, thisbytes);
-		if (err)
-			goto out_free_page;
-
-		written += thisbytes;
-		nbytes -= thisbytes;
-	}
-	err = 0;
-
-out_free_page:
-	free_page((unsigned long)buf);
-	return err;
-}
-
 void __iomem  *
 visorchannel_get_header(struct visorchannel *channel)
 {
@@ -490,129 +455,3 @@ visorchannel_signalinsert(struct visorchannel *channel, u32 queue, void *msg)
 	return rc;
 }
 EXPORT_SYMBOL_GPL(visorchannel_signalinsert);
-
-int
-visorchannel_signalqueue_slots_avail(struct visorchannel *channel, u32 queue)
-{
-	struct signal_queue_header sig_hdr;
-	u32 slots_avail, slots_used;
-	u32 head, tail;
-
-	if (!sig_read_header(channel, queue, &sig_hdr))
-		return 0;
-	head = sig_hdr.head;
-	tail = sig_hdr.tail;
-	if (head < tail)
-		head = head + sig_hdr.max_slots;
-	slots_used = head - tail;
-	slots_avail = sig_hdr.max_signals - slots_used;
-	return (int)slots_avail;
-}
-
-int
-visorchannel_signalqueue_max_slots(struct visorchannel *channel, u32 queue)
-{
-	struct signal_queue_header sig_hdr;
-
-	if (!sig_read_header(channel, queue, &sig_hdr))
-		return 0;
-	return (int)sig_hdr.max_signals;
-}
-
-static void
-sigqueue_debug(struct signal_queue_header *q, int which, struct seq_file *seq)
-{
-	seq_printf(seq, "Signal Queue #%d\n", which);
-	seq_printf(seq, "   VersionId          = %lu\n", (ulong)q->version);
-	seq_printf(seq, "   Type               = %lu\n", (ulong)q->chtype);
-	seq_printf(seq, "   oSignalBase        = %llu\n",
-		   (long long)q->sig_base_offset);
-	seq_printf(seq, "   SignalSize         = %lu\n", (ulong)q->signal_size);
-	seq_printf(seq, "   MaxSignalSlots     = %lu\n",
-		   (ulong)q->max_slots);
-	seq_printf(seq, "   MaxSignals         = %lu\n", (ulong)q->max_signals);
-	seq_printf(seq, "   FeatureFlags       = %-16.16Lx\n",
-		   (long long)q->features);
-	seq_printf(seq, "   NumSignalsSent     = %llu\n",
-		   (long long)q->num_sent);
-	seq_printf(seq, "   NumSignalsReceived = %llu\n",
-		   (long long)q->num_received);
-	seq_printf(seq, "   NumOverflows       = %llu\n",
-		   (long long)q->num_overflows);
-	seq_printf(seq, "   Head               = %lu\n", (ulong)q->head);
-	seq_printf(seq, "   Tail               = %lu\n", (ulong)q->tail);
-}
-
-void
-visorchannel_debug(struct visorchannel *channel, int num_queues,
-		   struct seq_file *seq, u32 off)
-{
-	u64 addr = 0;
-	ulong nbytes = 0, nbytes_region = 0;
-	struct channel_header hdr;
-	struct channel_header *phdr = &hdr;
-	int i = 0;
-	int errcode = 0;
-
-	if (!channel)
-		return;
-
-	addr = visorchannel_get_physaddr(channel);
-	nbytes_region = visorchannel_get_nbytes(channel);
-	errcode = visorchannel_read(channel, off,
-				    phdr, sizeof(struct channel_header));
-	if (errcode < 0) {
-		seq_printf(seq,
-			   "Read of channel header failed with errcode=%d)\n",
-			   errcode);
-		if (off == 0) {
-			phdr = &channel->chan_hdr;
-			seq_puts(seq, "(following data may be stale)\n");
-		} else {
-			return;
-		}
-	}
-	nbytes = (ulong)(phdr->size);
-	seq_printf(seq, "--- Begin channel @0x%-16.16Lx for 0x%lx bytes (region=0x%lx bytes) ---\n",
-		   addr + off, nbytes, nbytes_region);
-	seq_printf(seq, "Type            = %pUL\n", &phdr->chtype);
-	seq_printf(seq, "ZoneGuid        = %pUL\n", &phdr->zone_uuid);
-	seq_printf(seq, "Signature       = 0x%-16.16Lx\n",
-		   (long long)phdr->signature);
-	seq_printf(seq, "LegacyState     = %lu\n", (ulong)phdr->legacy_state);
-	seq_printf(seq, "SrvState        = %lu\n", (ulong)phdr->srv_state);
-	seq_printf(seq, "CliStateBoot    = %lu\n", (ulong)phdr->cli_state_boot);
-	seq_printf(seq, "CliStateOS      = %lu\n", (ulong)phdr->cli_state_os);
-	seq_printf(seq, "HeaderSize      = %lu\n", (ulong)phdr->header_size);
-	seq_printf(seq, "Size            = %llu\n", (long long)phdr->size);
-	seq_printf(seq, "Features        = 0x%-16.16llx\n",
-		   (long long)phdr->features);
-	seq_printf(seq, "PartitionHandle = 0x%-16.16llx\n",
-		   (long long)phdr->partition_handle);
-	seq_printf(seq, "Handle          = 0x%-16.16llx\n",
-		   (long long)phdr->handle);
-	seq_printf(seq, "VersionId       = %lu\n", (ulong)phdr->version_id);
-	seq_printf(seq, "oChannelSpace   = %llu\n",
-		   (long long)phdr->ch_space_offset);
-	if ((phdr->ch_space_offset == 0) || (errcode < 0))
-		;
-	else
-		for (i = 0; i < num_queues; i++) {
-			struct signal_queue_header q;
-
-			errcode = visorchannel_read(channel,
-						    off +
-						    phdr->ch_space_offset +
-						    (i * sizeof(q)),
-						    &q, sizeof(q));
-			if (errcode < 0) {
-				seq_printf(seq,
-					   "failed to read signal queue #%d from channel @0x%-16.16Lx errcode=%d\n",
-					   i, addr, errcode);
-				continue;
-			}
-			sigqueue_debug(&q, i, seq);
-		}
-	seq_printf(seq, "--- End   channel @0x%-16.16Lx for 0x%lx bytes ---\n",
-		   addr + off, nbytes);
-}
-- 
1.9.1

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

[PATCH 15/28] staging: unisys: Remove reference to unused STANDALONE_CLIENT

[David Kershner]

From: Bryan Thompson <bryan.thompson@unisys.com>

The STANDALONE_CLIENT define is no longer used by Unisys driver code.

Signed-off-by: Bryan Thompson <bryan.thompson@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index cbe240a..fb53a65 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -176,7 +176,6 @@ struct visor_device {
 
 #define to_visor_device(x) container_of(x, struct visor_device, device)
 
-#ifndef STANDALONE_CLIENT
 int visorbus_register_visor_driver(struct visor_driver *);
 void visorbus_unregister_visor_driver(struct visor_driver *);
 int visorbus_read_channel(struct visor_device *dev,
@@ -187,7 +186,6 @@ int visorbus_write_channel(struct visor_device *dev,
 			   unsigned long nbytes);
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
-#endif
 
 /* Note that for visorchannel_create()
  * <channel_bytes> and <guid> arguments may be 0 if we are a channel CLIENT.
-- 
1.9.1

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

[PATCH 16/28] staging: unisys: visorbus: remove unused parameter from function

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

The off parameter to visorchannel_create_guts() was never used, so it was
removed.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/visorchannel.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 43315c2..1bfbc06 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -60,8 +60,7 @@ struct visorchannel {
  */
 static struct visorchannel *
 visorchannel_create_guts(u64 physaddr, unsigned long channel_bytes,
-			 gfp_t gfp, unsigned long off,
-			 uuid_le guid, bool needs_lock)
+			 gfp_t gfp, uuid_le guid, bool needs_lock)
 {
 	struct visorchannel *channel;
 	int err;
@@ -145,7 +144,7 @@ struct visorchannel *
 visorchannel_create(u64 physaddr, unsigned long channel_bytes,
 		    gfp_t gfp, uuid_le guid)
 {
-	return visorchannel_create_guts(physaddr, channel_bytes, gfp, 0, guid,
+	return visorchannel_create_guts(physaddr, channel_bytes, gfp, guid,
 					false);
 }
 
@@ -153,7 +152,7 @@ struct visorchannel *
 visorchannel_create_with_lock(u64 physaddr, unsigned long channel_bytes,
 			      gfp_t gfp, uuid_le guid)
 {
-	return visorchannel_create_guts(physaddr, channel_bytes, gfp, 0, guid,
+	return visorchannel_create_guts(physaddr, channel_bytes, gfp, guid,
 					true);
 }
 
-- 
1.9.1

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

[PATCH 17/28] staging: unisys: visorbus: fix commenting in vbusdevinfo.h

[David Kershner]

From: David Binder <david.binder@unisys.com>

This patch ONLY touches comment lines, i.e., NO executable code is
affected.

* All functions worthy of documenting now use standard kerneldoc
  formatting.
* Multi-line comments were tweaked so as to use appropriate conventions.
* Minor typos were corrected.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/vbusdeviceinfo.h | 65 ++++++++++++++++--------
 1 file changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
index abdab4a..e6bfed1 100644
--- a/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
+++ b/drivers/staging/unisys/visorbus/vbusdeviceinfo.h
@@ -19,7 +19,8 @@
 
 #pragma pack(push, 1)		/* both GCC and VC now allow this pragma */
 
-/* An array of this struct is present in the channel area for each vbus.
+/*
+ * An array of this struct is present in the channel area for each vbus.
  * (See vbuschannel.h.)
  * It is filled in by the client side to provide info about the device
  * and driver from the client's perspective.
@@ -34,19 +35,28 @@ struct ultra_vbus_deviceinfo {
 
 #pragma pack(pop)
 
-/* Reads chars from the buffer at <src> for <srcmax> bytes, and writes to
- * the buffer at <p>, which is <remain> bytes long, ensuring never to
- * overflow the buffer at <p>, using the following rules:
- * - printable characters are simply copied from the buffer at <src> to the
- *   buffer at <p>
- * - intervening streaks of non-printable characters in the buffer at <src>
- *   are replaced with a single space in the buffer at <p>
+/**
+ * vbuschannel_sanitize_buffer() - remove non-printable chars from buffer
+ * @p: destination buffer where chars are written to
+ * @remain: number of bytes that can be written starting at #p
+ * @src: pointer to source buffer
+ * @srcmax: number of valid characters at #src
+ *
+ * Reads chars from the buffer at @src for @srcmax bytes, and writes to
+ * the buffer at @p, which is @remain bytes long, ensuring never to
+ * overflow the buffer at @p, using the following rules:
+ * - printable characters are simply copied from the buffer at @src to the
+ *   buffer at @p
+ * - intervening streaks of non-printable characters in the buffer at @src
+ *   are replaced with a single space in the buffer at @p
  * Note that we pay no attention to '\0'-termination.
- * Returns the number of bytes written to <p>.
  *
- * Pass <p> == NULL and <remain> == 0 for this special behavior.  In this
+ * Pass @p == NULL and @remain == 0 for this special behavior -- In this
  * case, we simply return the number of bytes that WOULD HAVE been written
- * to a buffer at <p>, had it been infinitely big.
+ * to a buffer at @p, had it been infinitely big.
+ *
+ * Return: the number of bytes written to @p (or WOULD HAVE been written to
+ *         @p, as described in the previous paragraph)
  */
 static inline int
 vbuschannel_sanitize_buffer(char *p, int remain, char *src, int srcmax)
@@ -92,14 +102,18 @@ vbuschannel_sanitize_buffer(char *p, int remain, char *src, int srcmax)
 		p++;  chars++;  remain--;	   \
 	} while (0)
 
-/* Converts the non-negative value at <num> to an ascii decimal string
- * at <p>, writing at most <remain> bytes.  Note there is NO '\0' termination
- * written to <p>.
+/**
+ * vbuschannel_itoa() - convert non-negative int to string
+ * @p: destination string
+ * @remain: max number of bytes that can be written to @p
+ * @num: input int to convert
+ *
+ * Converts the non-negative value at @num to an ascii decimal string
+ * at @p, writing at most @remain bytes.  Note there is NO '\0' termination
+ * written to @p.
  *
- * Returns the number of bytes written to <p>.
+ * Return: number of bytes written to @p
  *
- * Note that we create this function because we need to do this operation in
- * an environment-independent way (since we are in a common header file).
  */
 static inline int
 vbuschannel_itoa(char *p, int remain, int num)
@@ -141,13 +155,20 @@ vbuschannel_itoa(char *p, int remain, int num)
 	return digits;
 }
 
-/* Reads <devInfo>, and converts its contents to a printable string at <p>,
- * writing at most <remain> bytes.  Note there is NO '\0' termination
- * written to <p>.
+/**
+ * vbuschannel_devinfo_to_string() - format a struct ultra_vbus_deviceinfo
+ *                                   to a printable string
+ * @devinfo: the struct ultra_vbus_deviceinfo to format
+ * @p: destination string area
+ * @remain: size of destination string area in bytes
+ * @devix: the device index to be included in the output data, or -1 if no
+ *         device index is to be included
  *
- * Pass <devix> >= 0 if you want a device index presented.
+ * Reads @devInfo, and converts its contents to a printable string at @p,
+ * writing at most @remain bytes. Note there is NO '\0' termination
+ * written to @p.
  *
- * Returns the number of bytes written to <p>.
+ * Return: number of bytes written to @p
  */
 static inline int
 vbuschannel_devinfo_to_string(struct ultra_vbus_deviceinfo *devinfo,
-- 
1.9.1

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

[PATCH 18/28] staging: unisys: visorbus: fix commenting in visorbus_main.c

[David Kershner]

From: David Binder <david.binder@unisys.com>

This patch ONLY touches comment lines, i.e., NO executable code is
affected.

* All functions worthy of documenting now use standard kerneldoc
  formatting.
* Improper uses of kerneldoc formatting were converted to standard
  multi-line comments.
* Multi-line comments were tweaked so as to use appropriate conventions.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 420 ++++++++++++++++--------
 1 file changed, 289 insertions(+), 131 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 8c1754f9..2468264 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -44,11 +44,11 @@ static int visorbus_uevent(struct device *xdev, struct kobj_uevent_env *env);
 static int visorbus_match(struct device *xdev, struct device_driver *xdrv);
 static void fix_vbus_dev_info(struct visor_device *visordev);
 
-/*  BUS type attributes
- *
- *  define & implement display of bus attributes under
- *  /sys/bus/visorbus.
+/*
+ * BUS type attributes
  *
+ * define & implement display of bus attributes under
+ * /sys/bus/visorbus.
  */
 
 static ssize_t version_show(struct bus_type *bus, char *buf)
@@ -104,7 +104,8 @@ static const struct attribute_group *visorbus_dev_groups[] = {
 	NULL,
 };
 
-/** This describes the TYPE of bus.
+/*
+ * This describes the TYPE of bus.
  *  (Don't confuse this with an INSTANCE of the bus.)
  */
 struct bus_type visorbus_type = {
@@ -115,8 +116,8 @@ struct bus_type visorbus_type = {
 	.bus_groups = visorbus_bus_groups,
 };
 
-static long long bus_count;	/** number of bus instances */
-					/** ever-increasing */
+static long long bus_count;	/* number of bus instances */
+					/* ever-increasing */
 
 static void chipset_bus_create(struct visor_device *bus_info);
 static void chipset_bus_destroy(struct visor_device *bus_info);
@@ -125,8 +126,9 @@ static void chipset_device_destroy(struct visor_device *dev_info);
 static void chipset_device_pause(struct visor_device *dev_info);
 static void chipset_device_resume(struct visor_device *dev_info);
 
-/** These functions are implemented herein, and are called by the chipset
- *  driver to notify us about specific events.
+/*
+ * These functions are implemented herein, and are called by the chipset
+ * driver to notify us about specific events.
  */
 static struct visorchipset_busdev_notifiers chipset_notifiers = {
 	.bus_create = chipset_bus_create,
@@ -137,8 +139,9 @@ static struct visorchipset_busdev_notifiers chipset_notifiers = {
 	.device_resume = chipset_device_resume,
 };
 
-/** These functions are implemented in the chipset driver, and we call them
- *  herein when we want to acknowledge a specific event.
+/*
+ * These functions are implemented in the chipset driver, and we call them
+ * herein when we want to acknowledge a specific event.
  */
 static struct visorchipset_busdev_responders chipset_responders;
 
@@ -147,9 +150,9 @@ static struct ultra_vbus_deviceinfo chipset_driverinfo;
 /* filled in with info about this driver, wrt it servicing client busses */
 static struct ultra_vbus_deviceinfo clientbus_driverinfo;
 
-/** list of visor_device structs, linked via .list_all */
+/* list of visor_device structs, linked via .list_all */
 static LIST_HEAD(list_all_bus_instances);
-/** list of visor_device structs, linked via .list_all */
+/* list of visor_device structs, linked via .list_all */
 static LIST_HEAD(list_all_device_instances);
 
 static int
@@ -166,9 +169,14 @@ visorbus_uevent(struct device *xdev, struct kobj_uevent_env *env)
 	return 0;
 }
 
-/* This is called automatically upon adding a visor_device (device_add), or
- * adding a visor_driver (visorbus_register_visor_driver), and returns 1 iff the
- * provided driver can control the specified device.
+/**
+ * visorbus_match() - called automatically upon adding a visor_device
+ *                    (device_add), or adding a visor_driver
+ *                    (visorbus_register_visor_driver)
+ * @xdev: struct device for the device being matched
+ * @xdrv: struct device_driver for driver to match device against
+ *
+ * Return: 1 iff the provided driver can control the specified device
  */
 static int
 visorbus_match(struct device *xdev, struct device_driver *xdrv)
@@ -200,9 +208,11 @@ visorbus_match(struct device *xdev, struct device_driver *xdrv)
 	return 0;
 }
 
-/** This is called when device_unregister() is called for the bus device
- *  instance, after all other tasks involved with destroying the device
- *  are complete.
+/**
+ * visorbus_releae_busdevice() - called when device_unregister() is called for
+ *                               the bus device instance, after all other tasks
+ *                               involved with destroying the dev are complete
+ * @xdev: struct device for the bus being released
  */
 static void
 visorbus_release_busdevice(struct device *xdev)
@@ -212,8 +222,10 @@ visorbus_release_busdevice(struct device *xdev)
 	kfree(dev);
 }
 
-/** This is called when device_unregister() is called for each child
- *  device instance.
+/**
+ * visorbus_release_device() - called when device_unregister() is called for
+ *                             each child device instance
+ * @xdev: struct device for the visor device being released
  */
 static void
 visorbus_release_device(struct device *xdev)
@@ -227,9 +239,11 @@ visorbus_release_device(struct device *xdev)
 	kfree(dev);
 }
 
-/* begin implementation of specific channel attributes to appear under
-* /sys/bus/visorbus<x>/dev<y>/channel
-*/
+/*
+ * begin implementation of specific channel attributes to appear under
+ * /sys/bus/visorbus<x>/dev<y>/channel
+ */
+
 static ssize_t physaddr_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
@@ -334,15 +348,11 @@ static const struct attribute_group *visorbus_channel_groups[] = {
 
 /* end implementation of specific channel attributes */
 
-/*  BUS instance attributes
+/*
+ *  BUS instance attributes
  *
  *  define & implement display of bus attributes under
- *  /sys/bus/visorbus/busses/visorbus<n>.
- *
- *  This is a bit hoaky because the kernel does not yet have the infrastructure
- *  to separate bus INSTANCE attributes from bus TYPE attributes...
- *  so we roll our own.  See businst.c / businst.h.
- *
+ *  /sys/bus/visorbus/devices/visorbus<n>.
  */
 
 static ssize_t partition_handle_show(struct device *dev,
@@ -493,11 +503,11 @@ static const struct attribute_group *visorbus_groups[] = {
 		NULL
 };
 
-/*  DRIVER attributes
+/*
+ *  DRIVER attributes
  *
  *  define & implement display of driver attributes under
  *  /sys/bus/visorbus/drivers/<drivername>.
- *
  */
 
 static ssize_t
@@ -556,10 +566,20 @@ dev_stop_periodic_work(struct visor_device *dev)
 	put_device(&dev->device);
 }
 
-/** This is called automatically upon adding a visor_device (device_add), or
- *  adding a visor_driver (visorbus_register_visor_driver), but only after
- *  visorbus_match has returned 1 to indicate a successful match between
- *  driver and device.
+/**
+ * visordriver_probe_device() - handle new visor device coming online
+ * @xdev: struct device for the visor device being probed
+ *
+ * This is called automatically upon adding a visor_device (device_add), or
+ * adding a visor_driver (visorbus_register_visor_driver), but only after
+ * visorbus_match() has returned 1 to indicate a successful match between
+ * driver and device.
+ *
+ * If successful, a reference to the device will be held onto via get_device().
+ *
+ * Return: 0 if successful, meaning the function driver's probe() function
+ *         was successful with this device, otherwise a negative errno
+ *         value indicating failure reason
  */
 static int
 visordriver_probe_device(struct device *xdev)
@@ -588,9 +608,15 @@ visordriver_probe_device(struct device *xdev)
 	return res;
 }
 
-/** This is called when device_unregister() is called for each child device
- *  instance, to notify the appropriate visorbus_driver that the device is
- *  going away, and to decrease the reference count of the device.
+/**
+ * visordriver_remove_device() - handle visor device going away
+ * @xdev: struct device for the visor device being removed
+ *
+ * This is called when device_unregister() is called for each child device
+ * instance, to notify the appropriate visorbus function driver that the device
+ * is going away, and to decrease the reference count of the device.
+ *
+ * Return: 0 iff successful
  */
 static int
 visordriver_remove_device(struct device *xdev)
@@ -611,47 +637,54 @@ visordriver_remove_device(struct device *xdev)
 	return 0;
 }
 
-/** A particular type of visor driver calls this function to register
- *  the driver.  The caller MUST fill in the following fields within the
- *  #drv structure:
- *      name, version, owner, channel_types, probe, remove
+/**
+ * visorbus_register_visor_driver() - registers the provided visor driver
+ *                                    for handling one or more visor device
+ *                                    types (channel_types)
+ * @drv: the driver to register
  *
- *  Here's how the whole Linux bus / driver / device model works.
+ * A visor function driver calls this function to register
+ * the driver.  The caller MUST fill in the following fields within the
+ * #drv structure:
+ *     name, version, owner, channel_types, probe, remove
  *
- *  At system start-up, the visorbus kernel module is loaded, which registers
- *  visorbus_type as a bus type, using bus_register().
+ * Here's how the whole Linux bus / driver / device model works.
  *
- *  All kernel modules that support particular device types on a
- *  visorbus bus are loaded.  Each of these kernel modules calls
- *  visorbus_register_visor_driver() in their init functions, passing a
- *  visor_driver struct.  visorbus_register_visor_driver() in turn calls
- *  register_driver(&visor_driver.driver).  This .driver member is
- *  initialized with generic methods (like probe), whose sole responsibility
- *  is to act as a broker for the real methods, which are within the
- *  visor_driver struct.  (This is the way the subclass behavior is
- *  implemented, since visor_driver is essentially a subclass of the
- *  generic driver.)  Whenever a driver_register() happens, core bus code in
- *  the kernel does (see device_attach() in drivers/base/dd.c):
+ * At system start-up, the visorbus kernel module is loaded, which registers
+ * visorbus_type as a bus type, using bus_register().
  *
- *      for each dev associated with the bus (the bus that driver is on) that
- *      does not yet have a driver
- *          if bus.match(dev,newdriver) == yes_matched  ** .match specified
- *                                                 ** during bus_register().
- *              newdriver.probe(dev)  ** for visor drivers, this will call
- *                    ** the generic driver.probe implemented in visorbus.c,
- *                    ** which in turn calls the probe specified within the
- *                    ** struct visor_driver (which was specified by the
- *                    ** actual device driver as part of
- *                    ** visorbus_register_visor_driver()).
+ * All kernel modules that support particular device types on a
+ * visorbus bus are loaded.  Each of these kernel modules calls
+ * visorbus_register_visor_driver() in their init functions, passing a
+ * visor_driver struct.  visorbus_register_visor_driver() in turn calls
+ * register_driver(&visor_driver.driver).  This .driver member is
+ * initialized with generic methods (like probe), whose sole responsibility
+ * is to act as a broker for the real methods, which are within the
+ * visor_driver struct.  (This is the way the subclass behavior is
+ * implemented, since visor_driver is essentially a subclass of the
+ * generic driver.)  Whenever a driver_register() happens, core bus code in
+ * the kernel does (see device_attach() in drivers/base/dd.c):
  *
- *  The above dance also happens when a new device appears.
- *  So the question is, how are devices created within the system?
- *  Basically, just call device_add(dev).  See pci_bus_add_devices().
- *  pci_scan_device() shows an example of how to build a device struct.  It
- *  returns the newly-created struct to pci_scan_single_device(), who adds it
- *  to the list of devices at PCIBUS.devices.  That list of devices is what
- *  is traversed by pci_bus_add_devices().
+ *     for each dev associated with the bus (the bus that driver is on) that
+ *     does not yet have a driver
+ *         if bus.match(dev,newdriver) == yes_matched  ** .match specified
+ *                                                ** during bus_register().
+ *             newdriver.probe(dev)  ** for visor drivers, this will call
+ *                   ** the generic driver.probe implemented in visorbus.c,
+ *                   ** which in turn calls the probe specified within the
+ *                   ** struct visor_driver (which was specified by the
+ *                   ** actual device driver as part of
+ *                   ** visorbus_register_visor_driver()).
  *
+ * The above dance also happens when a new device appears.
+ * So the question is, how are devices created within the system?
+ * Basically, just call device_add(dev).  See pci_bus_add_devices().
+ * pci_scan_device() shows an example of how to build a device struct.  It
+ * returns the newly-created struct to pci_scan_single_device(), who adds it
+ * to the list of devices at PCIBUS.devices.  That list of devices is what
+ * is traversed by pci_bus_add_devices().
+ *
+ * Return: integer indicating success (zero) or failure (non-zero)
  */
 int visorbus_register_visor_driver(struct visor_driver *drv)
 {
@@ -666,7 +699,8 @@ int visorbus_register_visor_driver(struct visor_driver *drv)
 	drv->driver.remove = visordriver_remove_device;
 	drv->driver.owner = drv->owner;
 
-	/* driver_register does this:
+	/*
+	 * driver_register does this:
 	 *   bus_add_driver(drv)
 	 *   ->if (drv.bus)  ** (bus_type) **
 	 *       driver_attach(drv)
@@ -688,8 +722,12 @@ int visorbus_register_visor_driver(struct visor_driver *drv)
 }
 EXPORT_SYMBOL_GPL(visorbus_register_visor_driver);
 
-/** A particular type of visor driver calls this function to unregister
- *  the driver, i.e., within its module_exit function.
+/**
+ * visorbus_unregister_visor_driver() - unregisters the provided driver
+ * @drv: the driver to unregister
+ *
+ * A visor function driver calls this function to unregister the driver,
+ * i.e., within its module_exit function.
  */
 void
 visorbus_unregister_visor_driver(struct visor_driver *drv)
@@ -699,6 +737,19 @@ visorbus_unregister_visor_driver(struct visor_driver *drv)
 }
 EXPORT_SYMBOL_GPL(visorbus_unregister_visor_driver);
 
+/**
+ * visorbus_read_channel() - reads from the designated channel into
+ *                           the provided buffer
+ * @dev:    the device whose channel is read from
+ * @offset: the offset into the channel at which reading starts
+ * @dest:   the destination buffer that is written into from the channel
+ * @nbytes: the number of bytes to read from the channel
+ *
+ * If receiving a message, use the visorchannel_signalremove()
+ * function instead.
+ *
+ * Return: integer indicating success (zero) or failure (non-zero)
+ */
 int
 visorbus_read_channel(struct visor_device *dev, unsigned long offset,
 		      void *dest, unsigned long nbytes)
@@ -707,6 +758,19 @@ visorbus_read_channel(struct visor_device *dev, unsigned long offset,
 }
 EXPORT_SYMBOL_GPL(visorbus_read_channel);
 
+/**
+ * visorbus_write_channel() - writes the provided buffer into the designated
+ *                            channel
+ * @dev:    the device whose channel is written to
+ * @offset: the offset into the channel at which writing starts
+ * @src:    the source buffer that is written into the channel
+ * @nbytes: the number of bytes to write into the channel
+ *
+ * If sending a message, use the visorchannel_signalinsert()
+ * function instead.
+ *
+ * Return: integer indicating success (zero) or failure (non-zero)
+ */
 int
 visorbus_write_channel(struct visor_device *dev, unsigned long offset,
 		       void *src, unsigned long nbytes)
@@ -715,8 +779,13 @@ visorbus_write_channel(struct visor_device *dev, unsigned long offset,
 }
 EXPORT_SYMBOL_GPL(visorbus_write_channel);
 
-/** We don't really have a real interrupt, so for now we just call the
- *  interrupt function periodically...
+/**
+ * visorbus_enable_channel_interrupts() - enables interrupts on the
+ *                                        designated device
+ * @dev: the device on which to enable interrupts
+ *
+ * Currently we don't yet have a real interrupt, so for now we just call the
+ * interrupt function periodically via a timer.
  */
 void
 visorbus_enable_channel_interrupts(struct visor_device *dev)
@@ -725,6 +794,11 @@ visorbus_enable_channel_interrupts(struct visor_device *dev)
 }
 EXPORT_SYMBOL_GPL(visorbus_enable_channel_interrupts);
 
+/**
+ * visorbus_disable_channel_interrupts() - disables interrupts on the
+ *                                         designated device
+ * @dev: the device on which to disable interrupts
+ */
 void
 visorbus_disable_channel_interrupts(struct visor_device *dev)
 {
@@ -732,19 +806,28 @@ visorbus_disable_channel_interrupts(struct visor_device *dev)
 }
 EXPORT_SYMBOL_GPL(visorbus_disable_channel_interrupts);
 
-/** This is how everything starts from the device end.
- *  This function is called when a channel first appears via a ControlVM
- *  message.  In response, this function allocates a visor_device to
- *  correspond to the new channel, and attempts to connect it the appropriate
- *  driver.  If the appropriate driver is found, the visor_driver.probe()
- *  function for that driver will be called, and will be passed the new
- *  visor_device that we just created.
+/**
+ * create_visor_device() - create visor device as a result of receiving the
+ *                         controlvm device_create message for a new device
+ * @dev: a freshly-zeroed struct visor_device, containing only filled-in values
+ *       for chipset_bus_no and chipset_dev_no, that will be initialized
+ *
+ * This is how everything starts from the device end.
+ * This function is called when a channel first appears via a ControlVM
+ * message.  In response, this function allocates a visor_device to
+ * correspond to the new channel, and attempts to connect it the appropriate
+ * driver.  If the appropriate driver is found, the visor_driver.probe()
+ * function for that driver will be called, and will be passed the new
+ * visor_device that we just created.
  *
- *  It's ok if the appropriate driver is not yet loaded, because in that case
- *  the new device struct will just stick around in the bus' list of devices.
- *  When the appropriate driver calls visorbus_register_visor_driver(), the
- *  visor_driver.probe() for the new driver will be called with the new
- *  device.
+ * It's ok if the appropriate driver is not yet loaded, because in that case
+ * the new device struct will just stick around in the bus' list of devices.
+ * When the appropriate driver calls visorbus_register_visor_driver(), the
+ * visor_driver.probe() for the new driver will be called with the new
+ * device.
+ *
+ * Return: 0 if successful, otherwise the negative value returned by
+ *         device_add() indicating the reason for failure
  */
 static int
 create_visor_device(struct visor_device *dev)
@@ -767,14 +850,16 @@ create_visor_device(struct visor_device *dev)
 	dev->timer.data = (unsigned long)(dev);
 	dev->timer.function = dev_periodic_work;
 
-	/* bus_id must be a unique name with respect to this bus TYPE
+	/*
+	 * bus_id must be a unique name with respect to this bus TYPE
 	 * (NOT bus instance).  That's why we need to include the bus
 	 * number within the name.
 	 */
 	dev_set_name(&dev->device, "vbus%u:dev%u",
 		     chipset_bus_no, chipset_dev_no);
 
-	/*  device_add does this:
+	/*
+	 * device_add does this:
 	 *    bus_add_device(dev)
 	 *    ->device_attach(dev)
 	 *      ->for each driver drv registered on the bus that dev is on
@@ -834,13 +919,19 @@ get_vbus_header_info(struct visorchannel *chan,
 	return 0;
 }
 
-/* Write the contents of <info> to the struct
- * spar_vbus_channel_protocol.chp_info.
+/**
+ * write_vbus_chp_info() - write the contents of <info> to the struct
+ *                         spar_vbus_channel_protocol.chp_info
+ * @chan:     indentifies the s-Par channel that will be updated
+ * @hdr_info: used to find appropriate channel offset to write data
+ * @info:     contains the information to write
+ *
+ * Writes chipset info into the channel memory to be used for diagnostic
+ * purposes.
  *
- * Returns void since this is debug information and not needed for
+ * Returns no value since this is debug information and not needed for
  * device functionality.
  */
-
 static void
 write_vbus_chp_info(struct visorchannel *chan,
 		    struct spar_vbus_headerinfo *hdr_info,
@@ -854,13 +945,19 @@ write_vbus_chp_info(struct visorchannel *chan,
 	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
-/* Write the contents of <info> to the struct
- * spar_vbus_channel_protocol.bus_info.
+/**
+ * write_vbus_bus_info() - write the contents of <info> to the struct
+ *                         spar_vbus_channel_protocol.bus_info
+ * @chan:     indentifies the s-Par channel that will be updated
+ * @hdr_info: used to find appropriate channel offset to write data
+ * @info:     contains the information to write
+ *
+ * Writes bus info into the channel memory to be used for diagnostic
+ * purposes.
  *
- * Returns void since this is debug information and not needed for
+ * Returns no value since this is debug information and not needed for
  * device functionality.
  */
-
 static void
 write_vbus_bus_info(struct visorchannel *chan,
 		    struct spar_vbus_headerinfo *hdr_info,
@@ -874,10 +971,18 @@ write_vbus_bus_info(struct visorchannel *chan,
 	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
-/* Write the contents of <info> to the
- * struct spar_vbus_channel_protocol.dev_info[<devix>].
+/**
+ * write_vbus_dev_info() - write the contents of <info> to the struct
+ *                         spar_vbus_channel_protocol.dev_info[<devix>]
+ * @chan:     indentifies the s-Par channel that will be updated
+ * @hdr_info: used to find appropriate channel offset to write data
+ * @info:     contains the information to write
+ * @devix:    the relative device number (0..n-1) of the device on the bus
  *
- * Returns void since this is debug information and not needed for
+ * Writes device info into the channel memory to be used for diagnostic
+ * purposes.
+ *
+ * Returns no value since this is debug information and not needed for
  * device functionality.
  */
 static void
@@ -895,10 +1000,12 @@ write_vbus_dev_info(struct visorchannel *chan,
 	visorchannel_write(chan, off, info, sizeof(*info));
 }
 
-/* For a child device just created on a client bus, fill in
- * information about the driver that is controlling this device into
- * the the appropriate slot within the vbus channel of the bus
- * instance.
+/**
+ * fix_vbus_dev_info() - for a child device just created on a client bus, fill
+ *                       in information about the driver that is controlling
+ *                       this device into the the appropriate slot within the
+ *                       vbus channel of the bus instance
+ * @visordev: struct visor_device for the desired device
  */
 static void
 fix_vbus_dev_info(struct visor_device *visordev)
@@ -925,7 +1032,8 @@ fix_vbus_dev_info(struct visor_device *visordev)
 
 	visordrv = to_visor_driver(visordev->device.driver);
 
-	/* Within the list of device types (by GUID) that the driver
+	/*
+	 * Within the list of device types (by GUID) that the driver
 	 * says it supports, find out which one of those types matches
 	 * the type of this device, so that we can include the device
 	 * type name
@@ -944,15 +1052,21 @@ fix_vbus_dev_info(struct visor_device *visordev)
 			     visordrv->vertag);
 	write_vbus_dev_info(bdev->visorchannel, hdr_info, &dev_info, dev_no);
 
-	/* Re-write bus+chipset info, because it is possible that this
-	* was previously written by our evil counterpart, virtpci.
-	*/
+	/*
+	 * Re-write bus+chipset info, because it is possible that this
+	 * was previously written by our evil counterpart, virtpci.
+	 */
 	write_vbus_chp_info(bdev->visorchannel, hdr_info, &chipset_driverinfo);
 	write_vbus_bus_info(bdev->visorchannel, hdr_info,
 			    &clientbus_driverinfo);
 }
 
-/** Create a device instance for the visor bus itself.
+/**
+ * create_bus_instance() - create a device instance for the visor bus itself
+ * @dev: struct visor_device indicating the bus instance
+ *
+ * Return: 0 for success, otherwise negative errno value indicating reason for
+ *         failure
  */
 static int
 create_bus_instance(struct visor_device *dev)
@@ -993,12 +1107,15 @@ create_bus_instance(struct visor_device *dev)
 	return 0;
 }
 
-/** Remove a device instance for the visor bus itself.
+/**
+ * remove_bus_instance() - remove a device instance for the visor bus itself
+ * @dev: struct visor_device indentifying the bus to remove
  */
 static void
 remove_bus_instance(struct visor_device *dev)
 {
-	/* Note that this will result in the release method for
+	/*
+	 * Note that this will result in the release method for
 	 * dev->dev being called, which will call
 	 * visorbus_release_busdevice().  This has something to do with
 	 * the put_device() done in device_unregister(), but I have never
@@ -1015,8 +1132,11 @@ remove_bus_instance(struct visor_device *dev)
 	device_unregister(&dev->device);
 }
 
-/** Create and register the one-and-only one instance of
- *  the visor bus type (visorbus_type).
+/**
+ * create_bus_type() - create and register the one-and-only one instance of
+ *                     the visor bus type (visorbus_type)
+ * Return: 0 for success, otherwise negative errno value returned by
+ *         bus_register() indicating the reason for failure
  */
 static int
 create_bus_type(void)
@@ -1025,7 +1145,9 @@ create_bus_type(void)
 	return busreg_rc;
 }
 
-/** Remove the one-and-only one instance of the visor bus type (visorbus_type).
+/**
+ * remove_bus_type() - remove the one-and-only one instance of the visor bus
+ *                     type (visorbus_type)
  */
 static void
 remove_bus_type(void)
@@ -1033,7 +1155,8 @@ remove_bus_type(void)
 	bus_unregister(&visorbus_type);
 }
 
-/** Remove all child visor bus device instances.
+/**
+ * remove_all_visor_devices() - remove all child visor bus device instances
  */
 static void
 remove_all_visor_devices(void)
@@ -1108,9 +1231,14 @@ chipset_device_destroy(struct visor_device *dev_info)
 		(*chipset_responders.device_destroy) (dev_info, 0);
 }
 
-/* This is the callback function specified for a function driver, to
- * be called when a pending "pause device" operation has been
- * completed.
+/**
+ * pause_state_change_complete() - the callback function to be called by a
+ *                                 visorbus function driver when a
+ *                                 pending "pause device" operation has
+ *                                 completed
+ * @dev: struct visor_device identifying the paused device
+ * @status: 0 iff the pause state change completed successfully, otherwise
+ *          a negative errno value indicating the reason for failure
  */
 static void
 pause_state_change_complete(struct visor_device *dev, int status)
@@ -1129,9 +1257,14 @@ pause_state_change_complete(struct visor_device *dev, int status)
 	(*chipset_responders.device_pause) (dev, status);
 }
 
-/* This is the callback function specified for a function driver, to
- * be called when a pending "resume device" operation has been
- * completed.
+/**
+ * resume_state_change_complete() - the callback function to be called by a
+ *                                  visorbus function driver when a
+ *                                  pending "resume device" operation has
+ *                                  completed
+ * @dev: struct visor_device identifying the resumed device
+ * @status: 0 iff the resume state change completed successfully, otherwise
+ *          a negative errno value indicating the reason for failure
  */
 static void
 resume_state_change_complete(struct visor_device *dev, int status)
@@ -1143,16 +1276,24 @@ resume_state_change_complete(struct visor_device *dev, int status)
 	if (!chipset_responders.device_resume) /* this can never happen! */
 		return;
 
-	/* Notify the chipset driver that the resume is complete,
+	/*
+	 * Notify the chipset driver that the resume is complete,
 	 * which will presumably want to send some sort of response to
 	 * the initiator.
 	 */
 	(*chipset_responders.device_resume) (dev, status);
 }
 
-/* Tell the subordinate function driver for a specific device to pause
- * or resume that device.  Result is returned asynchronously via a
- * callback function.
+/**
+ * initiate_chipset_device_pause_resume() - start a pause or resume operation
+ *                                          for a visor device
+ * @dev: struct visor_device identifying the device being paused or resumed
+ * @is_pause: true to indicate pause operation, false to indicate resume
+ *
+ * Tell the subordinate function driver for a specific device to pause
+ * or resume that device.  Success/failure result is returned asynchronously
+ * via a callback function; see pause_state_change_complete() and
+ * resume_state_change_complete().
  */
 static void
 initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
@@ -1179,7 +1320,8 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 		return;
 	}
 
-	/* Note that even though both drv->pause() and drv->resume
+	/*
+	 * Note that even though both drv->pause() and drv->resume
 	 * specify a callback function, it is NOT necessary for us to
 	 * increment our local module usage count.  Reason is, there
 	 * is already a linkage dependency between child function
@@ -1219,12 +1361,28 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 	}
 }
 
+/**
+ * chipset_device_pause() - start a pause operation for a visor device
+ * @dev_info: struct visor_device identifying the device being paused
+ *
+ * Tell the subordinate function driver for a specific device to pause
+ * that device.  Success/failure result is returned asynchronously
+ * via a callback function; see pause_state_change_complete().
+ */
 static void
 chipset_device_pause(struct visor_device *dev_info)
 {
 	initiate_chipset_device_pause_resume(dev_info, true);
 }
 
+/**
+ * chipset_device_resume() - start a resume operation for a visor device
+ * @dev_info: struct visor_device identifying the device being resumed
+ *
+ * Tell the subordinate function driver for a specific device to resume
+ * that device.  Success/failure result is returned asynchronously
+ * via a callback function; see resume_state_change_complete().
+ */
 static void
 chipset_device_resume(struct visor_device *dev_info)
 {
-- 
1.9.1

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

[PATCH 19/28] staging: unisys: visorbus: fix visorchannel.c comments

[David Kershner]

From: David Binder <david.binder@unisys.com>

This patch ONLY touches comment lines, i.e., NO executable code is
affected.

Comments were fixed in visorchannel.c:
* All functions worthy of documenting now use standard kerneldoc
  formatting.
* Multi-line comments were tweaked so as to use appropriate conventions.
* Minor typos were corrected.
* Useless comments were removed.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: David Binder <david.binder@unisys.com>
---
 drivers/staging/unisys/visorbus/visorchannel.c | 83 ++++++++++++++++++++++----
 1 file changed, 71 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 1bfbc06..0ddfe05 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -15,7 +15,7 @@
  */
 
 /*
- *  This provides Supervisor channel communication primitives, which are
+ *  This provides s-Par channel communication primitives, which are
  *  independent of the mechanism used to access the channel data.
  */
 
@@ -55,8 +55,28 @@ struct visorchannel {
 	uuid_le inst;
 };
 
-/* Creates the struct visorchannel abstraction for a data area in memory,
- * but does NOT modify this data area.
+/**
+ * visorchannel_create_guts() - creates the struct visorchannel abstraction
+ *                              for a data area in memory, but does NOT modify
+ *                              this data area
+ * @physaddr:      physical address of start of channel
+ * @channel_bytes: size of the channel in bytes; this may 0 if the channel has
+ *                 already been initialized in memory (which is true for all
+ *                 channels provided to guest environments by the s-Par
+ *                 back-end), in which case the actual channel size will be
+ *                 read from the channel header in memory
+ * @gfp:           gfp_t to use when allocating memory for the data struct
+ * @guid:          uuid that identifies channel type; this may 0 if the channel
+ *                 has already been initialized in memory (which is true for all
+ *                 channels provided to guest environments by the s-Par
+ *                 back-end), in which case the actual channel guid will be
+ *                 read from the channel header in memory
+ * @needs_lock:    must specify true if you have multiple threads of execution
+ *                 that will be calling visorchannel methods of this
+ *                 visorchannel at the same time
+ *
+ * Return: pointer to visorchannel that was created if successful,
+ *         otherwise NULL
  */
 static struct visorchannel *
 visorchannel_create_guts(u64 physaddr, unsigned long channel_bytes,
@@ -77,7 +97,8 @@ visorchannel_create_guts(u64 physaddr, unsigned long channel_bytes,
 	spin_lock_init(&channel->insert_lock);
 	spin_lock_init(&channel->remove_lock);
 
-	/* Video driver constains the efi framebuffer so it will get a
+	/*
+	 * Video driver constains the efi framebuffer so it will get a
 	 * conflict resource when requesting its full mem region. Since
 	 * we are only using the efi framebuffer for video we can ignore
 	 * this. Remember that we haven't requested it so we don't try to
@@ -214,6 +235,12 @@ visorchannel_set_clientpartition(struct visorchannel *channel,
 	return 0;
 }
 
+/**
+ * visorchannel_get_uuid() - queries the UUID of the designated channel
+ * @channel: the channel to query
+ *
+ * Return: the UUID of the provided channel
+ */
 uuid_le
 visorchannel_get_uuid(struct visorchannel *channel)
 {
@@ -260,22 +287,25 @@ visorchannel_get_header(struct visorchannel *channel)
 	return (void __iomem *)&channel->chan_hdr;
 }
 
-/** Return offset of a specific SIGNAL_QUEUE_HEADER from the beginning of a
- *  channel header
+/*
+ * Return offset of a specific SIGNAL_QUEUE_HEADER from the beginning of a
+ * channel header
  */
 #define SIG_QUEUE_OFFSET(chan_hdr, q) \
 	((chan_hdr)->ch_space_offset + \
 	 ((q) * sizeof(struct signal_queue_header)))
 
-/** Return offset of a specific queue entry (data) from the beginning of a
- *  channel header
+/*
+ * Return offset of a specific queue entry (data) from the beginning of a
+ * channel header
  */
 #define SIG_DATA_OFFSET(chan_hdr, q, sig_hdr, slot) \
 	(SIG_QUEUE_OFFSET(chan_hdr, q) + (sig_hdr)->sig_base_offset + \
 	    ((slot) * (sig_hdr)->signal_size))
 
-/** Write the contents of a specific field within a SIGNAL_QUEUE_HEADER back
- *  into host memory
+/*
+ * Write the contents of a specific field within a SIGNAL_QUEUE_HEADER back
+ * into host memory
  */
 #define SIG_WRITE_FIELD(channel, queue, sig_hdr, FIELD)			 \
 	(visorchannel_write(channel,					 \
@@ -350,7 +380,8 @@ signalremove_inner(struct visorchannel *channel, u32 queue, void *msg)
 		return false;
 	sig_hdr.num_received++;
 
-	/* For each data field in SIGNAL_QUEUE_HEADER that was modified,
+	/*
+	 * For each data field in SIGNAL_QUEUE_HEADER that was modified,
 	 * update host memory.
 	 */
 	mb(); /* required for channel synch */
@@ -361,6 +392,15 @@ signalremove_inner(struct visorchannel *channel, u32 queue, void *msg)
 	return true;
 }
 
+/**
+ * visorchannel_signalremove() - removes a message from the designated
+ *                               channel/queue
+ * @channel: the channel the message will be removed from
+ * @queue:   the queue the message will be removed from
+ * @msg:     the message to remove
+ *
+ * Return: boolean indicating whether the removal succeeded or failed
+ */
 bool
 visorchannel_signalremove(struct visorchannel *channel, u32 queue, void *msg)
 {
@@ -379,6 +419,15 @@ visorchannel_signalremove(struct visorchannel *channel, u32 queue, void *msg)
 }
 EXPORT_SYMBOL_GPL(visorchannel_signalremove);
 
+/**
+ * visorchannel_signalempty() - checks if the designated channel/queue
+ *                              contains any messages
+ * @channel: the channel to query
+ * @queue:   the queue in the channel to query
+ *
+ * Return: boolean indicating whether any messages in the designated
+ *         channel/queue are present
+ */
 bool
 visorchannel_signalempty(struct visorchannel *channel, u32 queue)
 {
@@ -425,7 +474,8 @@ signalinsert_inner(struct visorchannel *channel, u32 queue, void *msg)
 
 	sig_hdr.num_sent++;
 
-	/* For each data field in SIGNAL_QUEUE_HEADER that was modified,
+	/*
+	 * For each data field in SIGNAL_QUEUE_HEADER that was modified,
 	 * update host memory.
 	 */
 	mb(); /* required for channel synch */
@@ -437,6 +487,15 @@ signalinsert_inner(struct visorchannel *channel, u32 queue, void *msg)
 	return true;
 }
 
+/**
+ * visorchannel_signalinsert() - inserts a message into the designated
+ *                               channel/queue
+ * @channel: the channel the message will be added to
+ * @queue:   the queue the message will be added to
+ * @msg:     the message to insert
+ *
+ * Return: boolean indicating whether the insertion succeeded or failed
+ */
 bool
 visorchannel_signalinsert(struct visorchannel *channel, u32 queue, void *msg)
 {
-- 
1.9.1

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

[PATCH 20/28] staging: unisys: visorbus: Rectify commenting in visorchipset.c

[David Kershner]

From: David Binder <david.binder@unisys.com>

Adds kerneldoc formatting to appropriate functions. Other multi-line
comments now use proper formatting.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorchipset.c | 292 ++++++++++++++++---------
 1 file changed, 190 insertions(+), 102 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index b5942c6..f8e1fa5 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -78,10 +78,11 @@ visorchipset_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-/* When the controlvm channel is idle for at least MIN_IDLE_SECONDS,
-* we switch to slow polling mode.  As soon as we get a controlvm
-* message, we switch back to fast polling mode.
-*/
+/*
+ * When the controlvm channel is idle for at least MIN_IDLE_SECONDS,
+ * we switch to slow polling mode. As soon as we get a controlvm
+ * message, we switch back to fast polling mode.
+ */
 #define MIN_IDLE_SECONDS 10
 static unsigned long poll_jiffies = POLLJIFFIES_CONTROLVMCHANNEL_FAST;
 /* when we got our last controlvm message */
@@ -112,7 +113,8 @@ static struct visorchannel *controlvm_channel;
 /* Manages the request payload in the controlvm channel */
 struct visor_controlvm_payload_info {
 	u8 *ptr;		/* pointer to base address of payload pool */
-	u64 offset;		/* offset from beginning of controlvm
+	u64 offset;		/*
+				 * offset from beginning of controlvm
 				 * channel to beginning of payload * pool
 				 */
 	u32 bytes;		/* number of bytes in payload pool */
@@ -120,15 +122,17 @@ struct visor_controlvm_payload_info {
 
 static struct visor_controlvm_payload_info controlvm_payload_info;
 
-/* The following globals are used to handle the scenario where we are unable to
- * offload the payload from a controlvm message due to memory requirements.  In
+/*
+ * The following globals are used to handle the scenario where we are unable to
+ * offload the payload from a controlvm message due to memory requirements. In
  * this scenario, we simply stash the controlvm message, then attempt to
  * process it again the next time controlvm_periodic_work() runs.
  */
 static struct controlvm_message controlvm_pending_msg;
 static bool controlvm_pending_msg_valid;
 
-/* This identifies a data buffer that has been received via a controlvm messages
+/*
+ * This identifies a data buffer that has been received via a controlvm messages
  * in a remote --> local CONTROLVM_TRANSMIT_FILE conversation.
  */
 struct putfile_buffer_entry {
@@ -136,13 +140,15 @@ struct putfile_buffer_entry {
 	struct parser_context *parser_ctx; /* points to input data buffer */
 };
 
-/* List of struct putfile_request *, via next_putfile_request member.
+/*
+ * List of struct putfile_request *, via next_putfile_request member.
  * Each entry in this list identifies an outstanding TRANSMIT_FILE
  * conversation.
  */
 static LIST_HEAD(putfile_request_list);
 
-/* This describes a buffer and its current state of transfer (e.g., how many
+/*
+ * This describes a buffer and its current state of transfer (e.g., how many
  * bytes have already been supplied as putfile data, and how many bytes are
  * remaining) for a putfile_request.
  */
@@ -154,8 +160,9 @@ struct putfile_active_buffer {
 };
 
 #define PUTFILE_REQUEST_SIG 0x0906101302281211
-/* This identifies a single remote --> local CONTROLVM_TRANSMIT_FILE
- * conversation.  Structs of this type are dynamically linked into
+/*
+ * This identifies a single remote --> local CONTROLVM_TRANSMIT_FILE
+ * conversation. Structs of this type are dynamically linked into
  * <Putfile_request_list>.
  */
 struct putfile_request {
@@ -167,7 +174,8 @@ struct putfile_request {
 	/* link to next struct putfile_request */
 	struct list_head next_putfile_request;
 
-	/* head of putfile_buffer_entry list, which describes the data to be
+	/*
+	 * head of putfile_buffer_entry list, which describes the data to be
 	 * supplied as putfile data;
 	 * - this list is added to when controlvm messages come in that supply
 	 * file data
@@ -183,11 +191,13 @@ struct putfile_request {
 	/* data not yet read within current putfile_buffer_entry */
 	struct putfile_active_buffer active_buf;
 
-	/* <0 = failed, 0 = in-progress, >0 = successful; */
-	/* note that this must be set with req_list_lock, and if you set <0, */
-	/* it is your responsibility to also free up all of the other objects */
-	/* in this struct (like input_buffer_list, active_buf.parser_ctx) */
-	/* before releasing the lock */
+	/*
+	 * <0 = failed, 0 = in-progress, >0 = successful;
+	 * note that this must be set with req_list_lock, and if you set <0,
+	 * it is your responsibility to also free up all of the other objects
+	 * in this struct (like input_buffer_list, active_buf.parser_ctx)
+	 * before releasing the lock
+	 */
 	int completion_status;
 };
 
@@ -202,9 +212,6 @@ static LIST_HEAD(parahotplug_request_list);
 static DEFINE_SPINLOCK(parahotplug_request_list_lock);	/* lock for above */
 static void parahotplug_process_list(void);
 
-/* Manages the info for a CONTROLVM_DUMP_CAPTURESTATE /
- * CONTROLVM_REPORTEVENT.
- */
 static struct visorchipset_busdev_notifiers busdev_notifiers;
 
 static void bus_create_response(struct visor_device *p, int response);
@@ -226,7 +233,7 @@ static struct visorchipset_busdev_responders busdev_responders = {
 };
 
 /* info for /dev/visorchipset */
-static dev_t major_dev = -1; /**< indicates major num for device */
+static dev_t major_dev = -1; /*< indicates major num for device */
 
 /* prototypes for attributes */
 static ssize_t toolaction_show(struct device *dev,
@@ -396,8 +403,9 @@ parser_id_get(struct parser_context *ctx)
 	return phdr->id;
 }
 
-/** Describes the state from the perspective of which controlvm messages have
- *  been received for a bus or device.
+/*
+ * Describes the state from the perspective of which controlvm messages have
+ * been received for a bus or device.
  */
 
 enum PARSER_WHICH_STRING {
@@ -722,14 +730,16 @@ chipset_init(struct controlvm_message *inmsg)
 	chipset_inited = 1;
 	POSTCODE_LINUX_2(CHIPSET_INIT_EXIT_PC, POSTCODE_SEVERITY_INFO);
 
-	/* Set features to indicate we support parahotplug (if Command
+	/*
+	 * Set features to indicate we support parahotplug (if Command
 	 * also supports it).
 	 */
 	features =
 	    inmsg->cmd.init_chipset.
 	    features & ULTRA_CHIPSET_FEATURE_PARA_HOTPLUG;
 
-	/* Set the "reply" bit so Command knows this is a
+	/*
+	 * Set the "reply" bit so Command knows this is a
 	 * features-aware driver.
 	 */
 	features |= ULTRA_CHIPSET_FEATURE_REPLY;
@@ -920,8 +930,10 @@ bus_epilog(struct visor_device *bus_info,
 	down(&notifier_lock);
 
 	if (!bus_info) {
-		/* relying on a valid passed in response code */
-		/* be lazy and re-use msg_hdr for this failure, is this ok?? */
+		/*
+		 * relying on a valid passed in response code
+		 * be lazy and re-use msg_hdr for this failure, is this ok??
+		 */
 		pmsg_hdr = msg_hdr;
 		goto out_respond_and_unlock;
 	}
@@ -984,8 +996,10 @@ device_epilog(struct visor_device *dev_info,
 
 	down(&notifier_lock);
 	if (!dev_info) {
-		/* relying on a valid passed in response code */
-		/* be lazy and re-use msg_hdr for this failure, is this ok?? */
+		/*
+		 * relying on a valid passed in response code
+		 * be lazy and re-use msg_hdr for this failure, is this ok??
+		 */
 		pmsg_hdr = msg_hdr;
 		goto out_respond_and_unlock;
 	}
@@ -1031,7 +1045,8 @@ device_epilog(struct visor_device *dev_info,
 			else if (state.alive == segment_state_standby.alive &&
 				 state.operating ==
 				 segment_state_standby.operating) {
-				/* technically this is standby case
+				/*
+				 * technically this is standby case
 				 * where server is lost
 				 */
 				if (notifiers->device_pause) {
@@ -1300,11 +1315,19 @@ my_device_destroy(struct controlvm_message *inmsg)
 			      inmsg->hdr.flags.response_expected == 1, 1);
 }
 
-/* When provided with the physical address of the controlvm channel
+/**
+ * initialize_controlvm_payload_info() - init controlvm_payload_info struct
+ * @phys_addr: the physical address of controlvm channel
+ * @offset:    the offset to payload
+ * @bytes:     the size of the payload in bytes
+ * @info:      the returning valid struct
+ *
+ * When provided with the physical address of the controlvm channel
  * (phys_addr), the offset to the payload area we need to manage
  * (offset), and the size of this payload area (bytes), fills in the
- * controlvm_payload_info struct.  Returns true for success or false
- * for failure.
+ * controlvm_payload_info struct.
+ *
+ * Return: CONTROLVM_RESP_SUCCESS for success or a negative for failure
  */
 static int
 initialize_controlvm_payload_info(u64 phys_addr, u64 offset, u32 bytes,
@@ -1368,8 +1391,12 @@ initialize_controlvm_payload(void)
 					  &controlvm_payload_info);
 }
 
-/*  Send ACTION=online for DEVPATH=/sys/devices/platform/visorchipset.
- *  Returns CONTROLVM_RESP_xxx code.
+/**
+ * visorchipset_chipset_ready() - sends chipset_ready action
+ *
+ * Send ACTION=online for DEVPATH=/sys/devices/platform/visorchipset.
+ *
+ * Return: CONTROLVM_RESP_SUCCESS
  */
 static int
 visorchipset_chipset_ready(void)
@@ -1390,8 +1417,12 @@ visorchipset_chipset_selftest(void)
 	return CONTROLVM_RESP_SUCCESS;
 }
 
-/*  Send ACTION=offline for DEVPATH=/sys/devices/platform/visorchipset.
- *  Returns CONTROLVM_RESP_xxx code.
+/**
+ * visorchipset_chipset_notready() - sends chipset_notready action
+ *
+ * Send ACTION=offline for DEVPATH=/sys/devices/platform/visorchipset.
+ *
+ * Return: CONTROLVM_RESP_SUCCESS
  */
 static int
 visorchipset_chipset_notready(void)
@@ -1433,8 +1464,13 @@ chipset_notready(struct controlvm_message_header *msg_hdr)
 		controlvm_respond(msg_hdr, rc);
 }
 
-/* This is your "one-stop" shop for grabbing the next message from the
- * CONTROLVM_QUEUE_EVENT queue in the controlvm channel.
+/**
+ * read_controlvm_event() - retreives the next message from the
+ *                          CONTROLVM_QUEUE_EVENT queue in the controlvm
+ *                          channel
+ * @msg: pointer to the retrieved message
+ *
+ * Return: true if a valid message was retrieved or false otherwise
  */
 static bool
 read_controlvm_event(struct controlvm_message *msg)
@@ -1450,13 +1486,13 @@ read_controlvm_event(struct controlvm_message *msg)
 }
 
 /*
- * The general parahotplug flow works as follows.  The visorchipset
+ * The general parahotplug flow works as follows. The visorchipset
  * driver receives a DEVICE_CHANGESTATE message from Command
- * specifying a physical device to enable or disable.  The CONTROLVM
+ * specifying a physical device to enable or disable. The CONTROLVM
  * message handler calls parahotplug_process_message, which then adds
  * the message to a global list and kicks off a udev event which
  * causes a user level script to enable or disable the specified
- * device.  The udev script then writes to
+ * device. The udev script then writes to
  * /proc/visorchipset/parahotplug, which causes parahotplug_proc_write
  * to get called, at which point the appropriate CONTROLVM message is
  * retrieved from the list and responded to.
@@ -1464,9 +1500,11 @@ read_controlvm_event(struct controlvm_message *msg)
 
 #define PARAHOTPLUG_TIMEOUT_MS 2000
 
-/*
- * Generate unique int to match an outstanding CONTROLVM message with a
- * udev script /proc response
+/**
+ * parahotplug_next_id() - generate unique int to match an outstanding CONTROLVM
+ *                         message with a udev script /proc response
+ *
+ * Return: a unique integer value
  */
 static int
 parahotplug_next_id(void)
@@ -1476,9 +1514,12 @@ parahotplug_next_id(void)
 	return atomic_inc_return(&id);
 }
 
-/*
- * Returns the time (in jiffies) when a CONTROLVM message on the list
- * should expire -- PARAHOTPLUG_TIMEOUT_MS in the future
+/**
+ * parahotplug_next_expiration() - returns the time (in jiffies) when a
+ *                                 CONTROLVM message on the list should expire
+ *                                 -- PARAHOTPLUG_TIMEOUT_MS in the future
+ *
+ * Return: expected expiration time (in jiffies)
  */
 static unsigned long
 parahotplug_next_expiration(void)
@@ -1486,9 +1527,13 @@ parahotplug_next_expiration(void)
 	return jiffies + msecs_to_jiffies(PARAHOTPLUG_TIMEOUT_MS);
 }
 
-/*
- * Create a parahotplug_request, which is basically a wrapper for a
- * CONTROLVM_MESSAGE that we can stick on a list
+/**
+ * parahotplug_request_create() - create a parahotplug_request, which is
+ *                                basically a wrapper for a CONTROLVM_MESSAGE
+ *                                that we can stick on a list
+ * @msg: the message to insert in the request
+ *
+ * Return: the request containing the provided message
  */
 static struct parahotplug_request *
 parahotplug_request_create(struct controlvm_message *msg)
@@ -1506,8 +1551,9 @@ parahotplug_request_create(struct controlvm_message *msg)
 	return req;
 }
 
-/*
- * Free a parahotplug_request.
+/**
+ * parahotplug_request_destroy() - free a parahotplug_request
+ * @req: the request to deallocate
  */
 static void
 parahotplug_request_destroy(struct parahotplug_request *req)
@@ -1515,10 +1561,12 @@ parahotplug_request_destroy(struct parahotplug_request *req)
 	kfree(req);
 }
 
-/*
- * Cause uevent to run the user level script to do the disable/enable
- * specified in (the CONTROLVM message in) the specified
- * parahotplug_request
+/**
+ * parahotplug_request_kickoff() - initiate parahotplug request
+ * @req: the request to initiate
+ *
+ * Cause uevent to run the user level script to do the disable/enable specified
+ * in the parahotplug_request.
  */
 static void
 parahotplug_request_kickoff(struct parahotplug_request *req)
@@ -1545,9 +1593,9 @@ parahotplug_request_kickoff(struct parahotplug_request *req)
 			   envp);
 }
 
-/*
- * Remove any request from the list that's been on there too long and
- * respond with an error.
+/**
+ * parahotplug_process_list() - remove any request from the list that's been on
+ *                              there too long and respond with an error
  */
 static void
 parahotplug_process_list(void)
@@ -1576,10 +1624,16 @@ parahotplug_process_list(void)
 	spin_unlock(&parahotplug_request_list_lock);
 }
 
-/*
+/**
+ * parahotplug_request_complete() - mark request as complete
+ * @id:     the id of the request
+ * @active: indicates whether the request is assigned to active partition
+ *
  * Called from the /proc handler, which means the user script has
- * finished the enable/disable.  Find the matching identifier, and
+ * finished the enable/disable. Find the matching identifier, and
  * respond to the CONTROLVM message with success.
+ *
+ * Return: 0 on success or -EINVAL on failure
  */
 static int
 parahotplug_request_complete(int id, u16 active)
@@ -1594,7 +1648,8 @@ parahotplug_request_complete(int id, u16 active)
 		struct parahotplug_request *req =
 		    list_entry(pos, struct parahotplug_request, list);
 		if (req->id == id) {
-			/* Found a match.  Remove it from the list and
+			/*
+			 * Found a match. Remove it from the list and
 			 * respond.
 			 */
 			list_del(pos);
@@ -1613,8 +1668,10 @@ parahotplug_request_complete(int id, u16 active)
 	return -EINVAL;
 }
 
-/*
- * Enables or disables a PCI device by kicking off a udev script
+/**
+ * parahotplug_process_message() - enables or disables a PCI device by kicking
+ *                                 off a udev script
+ * @inmsg: the message indicating whether to enable or disable
  */
 static void
 parahotplug_process_message(struct controlvm_message *inmsg)
@@ -1627,14 +1684,16 @@ parahotplug_process_message(struct controlvm_message *inmsg)
 		return;
 
 	if (inmsg->cmd.device_change_state.state.active) {
-		/* For enable messages, just respond with success
-		* right away.  This is a bit of a hack, but there are
-		* issues with the early enable messages we get (with
-		* either the udev script not detecting that the device
-		* is up, or not getting called at all).  Fortunately
-		* the messages that get lost don't matter anyway, as
-		* devices are automatically enabled at
-		* initialization.
+		/*
+		 * For enable messages, just respond with success
+		 * right away. This is a bit of a hack, but there are
+		 * issues with the early enable messages we get (with
+		 * either the udev script not detecting that the device
+		 * is up, or not getting called at all). Fortunately
+		 * the messages that get lost don't matter anyway, as
+		 *
+		 * devices are automatically enabled at
+		 * initialization.
 		*/
 		parahotplug_request_kickoff(req);
 		controlvm_respond_physdev_changestate
@@ -1643,11 +1702,12 @@ parahotplug_process_message(struct controlvm_message *inmsg)
 			 inmsg->cmd.device_change_state.state);
 		parahotplug_request_destroy(req);
 	} else {
-		/* For disable messages, add the request to the
-		* request list before kicking off the udev script.  It
-		* won't get responded to until the script has
-		* indicated it's done.
-		*/
+		/*
+		 * For disable messages, add the request to the
+		 * request list before kicking off the udev script. It
+		 * won't get responded to until the script has
+		 * indicated it's done.
+		 */
 		spin_lock(&parahotplug_request_list_lock);
 		list_add_tail(&req->list, &parahotplug_request_list);
 		spin_unlock(&parahotplug_request_list_lock);
@@ -1656,8 +1716,12 @@ parahotplug_process_message(struct controlvm_message *inmsg)
 	}
 }
 
-/* Process a controlvm message.
- * Return result:
+/**
+ * handle_command() - process a controlvm message
+ * @inmsg:        the message to process
+ * @channel_addr: address of the controlvm channel
+ *
+ * Return:
  *    false - this function will return false only in the case where the
  *            controlvm message was NOT processed, but processing must be
  *            retried before reading the next controlvm message; a
@@ -1665,7 +1729,7 @@ parahotplug_process_message(struct controlvm_message *inmsg)
  *            the allocation of memory in which to copy out controlvm
  *            payload data
  *    true  - processing of the controlvm message completed,
- *            either successfully or with an error.
+ *            either successfully or with an error
  */
 static bool
 handle_command(struct controlvm_message inmsg, u64 channel_addr)
@@ -1684,8 +1748,9 @@ handle_command(struct controlvm_message inmsg, u64 channel_addr)
 	parm_addr = channel_addr + inmsg.hdr.payload_vm_offset;
 	parm_bytes = inmsg.hdr.payload_bytes;
 
-	/* Parameter and channel addresses within test messages actually lie
-	 * within our OS-controlled memory.  We need to know that, because it
+	/*
+	 * Parameter and channel addresses within test messages actually lie
+	 * within our OS-controlled memory. We need to know that, because it
 	 * makes a difference in how we compute the virtual address.
 	 */
 	if (parm_addr && parm_bytes) {
@@ -1726,8 +1791,10 @@ handle_command(struct controlvm_message inmsg, u64 channel_addr)
 		if (cmd->device_change_state.flags.phys_device) {
 			parahotplug_process_message(&inmsg);
 		} else {
-			/* save the hdr and cmd structures for later use */
-			/* when sending back the response to Command */
+			/*
+			 * save the hdr and cmd structures for later use
+			 * when sending back the response to Command
+			 */
 			my_device_changestate(&inmsg);
 			g_devicechangestate_packet = inmsg.cmd;
 			break;
@@ -1809,10 +1876,11 @@ controlvm_periodic_work(struct work_struct *work)
 		;
 	if (!got_command) {
 		if (controlvm_pending_msg_valid) {
-			/* we throttled processing of a prior
-			* msg, so try to process it again
-			* rather than reading a new one
-			*/
+			/*
+			 * we throttled processing of a prior
+			 * msg, so try to process it again
+			 * rather than reading a new one
+			 */
 			inmsg = controlvm_pending_msg;
 			controlvm_pending_msg_valid = false;
 			got_command = true;
@@ -1829,12 +1897,13 @@ controlvm_periodic_work(struct work_struct *work)
 				   (controlvm_channel)))
 			got_command = read_controlvm_event(&inmsg);
 		else {
-			/* this is a scenario where throttling
-			* is required, but probably NOT an
-			* error...; we stash the current
-			* controlvm msg so we will attempt to
-			* reprocess it on our next loop
-			*/
+			/*
+			 * this is a scenario where throttling
+			 * is required, but probably NOT an
+			 * error...; we stash the current
+			 * controlvm msg so we will attempt to
+			 * reprocess it on our next loop
+			 */
 			handle_command_failed = true;
 			controlvm_pending_msg = inmsg;
 			controlvm_pending_msg_valid = true;
@@ -1848,10 +1917,11 @@ cleanup:
 
 	if (time_after(jiffies,
 		       most_recent_message_jiffies + (HZ * MIN_IDLE_SECONDS))) {
-		/* it's been longer than MIN_IDLE_SECONDS since we
-		* processed our last controlvm message; slow down the
-		* polling
-		*/
+		/*
+		 * it's been longer than MIN_IDLE_SECONDS since we
+		 * processed our last controlvm message; slow down the
+		 * polling
+		 */
 		if (poll_jiffies != POLLJIFFIES_CONTROLVMCHANNEL_SLOW)
 			poll_jiffies = POLLJIFFIES_CONTROLVMCHANNEL_SLOW;
 	} else {
@@ -2024,9 +2094,18 @@ device_resume_response(struct visor_device *dev_info, int response)
 	dev_info->pending_msg_hdr = NULL;
 }
 
-/* The parahotplug/devicedisabled interface gets called by our support script
+/**
+ * devicedisabled_store() - disables the hotplug device
+ * @dev:   sysfs interface variable not utilized in this function
+ * @attr:  sysfs interface variable not utilized in this function
+ * @buf:   buffer containing the device id
+ * @count: the size of the buffer
+ *
+ * The parahotplug/devicedisabled interface gets called by our support script
  * when an SR-IOV device has been shut down. The ID is passed to the script
  * and then passed back when the device has been removed.
+ *
+ * Return: the size of the buffer for success or negative for error
  */
 static ssize_t devicedisabled_store(struct device *dev,
 				    struct device_attribute *attr,
@@ -2044,9 +2123,18 @@ static ssize_t devicedisabled_store(struct device *dev,
 	return count;
 }
 
-/* The parahotplug/deviceenabled interface gets called by our support script
+/**
+ * deviceenabled_store() - enables the hotplug device
+ * @dev:   sysfs interface variable not utilized in this function
+ * @attr:  sysfs interface variable not utilized in this function
+ * @buf:   buffer containing the device id
+ * @count: the size of the buffer
+ *
+ * The parahotplug/deviceenabled interface gets called by our support script
  * when an SR-IOV device has been recovered. The ID is passed to the script
  * and then passed back when the device has been brought back up.
+ *
+ * Return: the size of the buffer for success or negative for error
  */
 static ssize_t deviceenabled_store(struct device *dev,
 				   struct device_attribute *attr,
-- 
1.9.1

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

[PATCH 21/28] staging: unisys: visorbus: Move visorbus-unique functions to private header

[David Kershner]

From: David Binder <david.binder@unisys.com>

Moves function prototypes that are unique to visorbus from
include/visorbus.h to visorbus/visorbus_private.h.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h          | 25 --------------------
 drivers/staging/unisys/visorbus/visorbus_private.h | 27 ++++++++++++++++++++++
 drivers/staging/unisys/visorbus/visorchannel.c     |  1 +
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index fb53a65..666a04e 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -187,37 +187,12 @@ int visorbus_write_channel(struct visor_device *dev,
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
 
-/* Note that for visorchannel_create()
- * <channel_bytes> and <guid> arguments may be 0 if we are a channel CLIENT.
- * In this case, the values can simply be read from the channel header.
- */
-struct visorchannel *visorchannel_create(u64 physaddr,
-					 unsigned long channel_bytes,
-					 gfp_t gfp, uuid_le guid);
-struct visorchannel *visorchannel_create_with_lock(u64 physaddr,
-						   unsigned long channel_bytes,
-						   gfp_t gfp, uuid_le guid);
-void visorchannel_destroy(struct visorchannel *channel);
-int visorchannel_read(struct visorchannel *channel, ulong offset,
-		      void *local, ulong nbytes);
-int visorchannel_write(struct visorchannel *channel, ulong offset,
-		       void *local, ulong nbytes);
 bool visorchannel_signalremove(struct visorchannel *channel, u32 queue,
 			       void *msg);
 bool visorchannel_signalinsert(struct visorchannel *channel, u32 queue,
 			       void *msg);
 bool visorchannel_signalempty(struct visorchannel *channel, u32 queue);
-
-u64 visorchannel_get_physaddr(struct visorchannel *channel);
-ulong visorchannel_get_nbytes(struct visorchannel *channel);
-char *visorchannel_id(struct visorchannel *channel, char *s);
-char *visorchannel_zoneid(struct visorchannel *channel, char *s);
-u64 visorchannel_get_clientpartition(struct visorchannel *channel);
-int visorchannel_set_clientpartition(struct visorchannel *channel,
-				     u64 partition_handle);
 uuid_le visorchannel_get_uuid(struct visorchannel *channel);
-char *visorchannel_uuid_id(uuid_le *guid, char *s);
-void __iomem *visorchannel_get_header(struct visorchannel *channel);
 
 #define BUS_ROOT_DEVICE		UINT_MAX
 struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/staging/unisys/visorbus/visorbus_private.h
index 39edd20..31b5ca9 100644
--- a/drivers/staging/unisys/visorbus/visorbus_private.h
+++ b/drivers/staging/unisys/visorbus/visorbus_private.h
@@ -65,4 +65,31 @@ visorchipset_register_busdev(
 /* visorbus init and exit functions */
 int visorbus_init(void);
 void visorbus_exit(void);
+
+/* Visorchannel access functions */
+
+/* Note that for visorchannel_create()
+ * <channel_bytes> and <guid> arguments may be 0 if we are a channel CLIENT.
+ * In this case, the values can simply be read from the channel header.
+ */
+struct visorchannel *visorchannel_create(u64 physaddr,
+					 unsigned long channel_bytes,
+					 gfp_t gfp, uuid_le guid);
+struct visorchannel *visorchannel_create_with_lock(u64 physaddr,
+						   unsigned long channel_bytes,
+						   gfp_t gfp, uuid_le guid);
+void visorchannel_destroy(struct visorchannel *channel);
+int visorchannel_read(struct visorchannel *channel, ulong offset,
+		      void *local, ulong nbytes);
+int visorchannel_write(struct visorchannel *channel, ulong offset,
+		       void *local, ulong nbytes);
+u64 visorchannel_get_physaddr(struct visorchannel *channel);
+ulong visorchannel_get_nbytes(struct visorchannel *channel);
+char *visorchannel_id(struct visorchannel *channel, char *s);
+char *visorchannel_zoneid(struct visorchannel *channel, char *s);
+u64 visorchannel_get_clientpartition(struct visorchannel *channel);
+int visorchannel_set_clientpartition(struct visorchannel *channel,
+				     u64 partition_handle);
+char *visorchannel_uuid_id(uuid_le *guid, char *s);
+void __iomem *visorchannel_get_header(struct visorchannel *channel);
 #endif
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 0ddfe05..fbae66e 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -25,6 +25,7 @@
 #include "version.h"
 #include "visorbus.h"
 #include "controlvmchannel.h"
+#include "visorbus_private.h"
 
 #define MYDRVNAME "visorchannel"
 
-- 
1.9.1

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

[PATCH 22/28] staging: unisys: visorbus: rectify kerneldoc comment for struct

[David Kershner]

From: David Binder <david.binder@unisys.com>

Fixes the kerneldoc comment for struct visor_device - the struct members
were not listed with the appropriate @ prefix.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h | 34 +++++++++++++++----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 666a04e..c836c8d 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -121,33 +121,33 @@ struct visor_driver {
 /**
  * struct visor_device - A device type for things "plugged" into the visorbus
  * bus
- * visorchannel:		Points to the channel that the device is
+ * @visorchannel:		Points to the channel that the device is
  *				associated with.
- * channel_type_guid:		Identifies the channel type to the bus driver.
- * device:			Device struct meant for use by the bus driver
+ * @channel_type_guid:		Identifies the channel type to the bus driver.
+ * @device:			Device struct meant for use by the bus driver
  *				only.
- * list_all:			Used by the bus driver to enumerate devices.
- * timer:		        Timer fired periodically to do interrupt-type
+ * @list_all:			Used by the bus driver to enumerate devices.
+ * @timer:		        Timer fired periodically to do interrupt-type
  *				activity.
- * being_removed:		Indicates that the device is being removed from
+ * @being_removed:		Indicates that the device is being removed from
  *				the bus. Private bus driver use only.
- * visordriver_callback_lock:	Used by the bus driver to lock when handling
+ * @visordriver_callback_lock:	Used by the bus driver to lock when handling
  *				channel events.
- * pausing:			Indicates that a change towards a paused state.
+ * @pausing:			Indicates that a change towards a paused state.
  *				is in progress. Only modified by the bus driver.
- * resuming:			Indicates that a change towards a running state
+ * @resuming:			Indicates that a change towards a running state
  *				is in progress. Only modified by the bus driver.
- * chipset_bus_no:		Private field used by the bus driver.
- * chipset_dev_no:		Private field used the bus driver.
- * state:			Used to indicate the current state of the
+ * @chipset_bus_no:		Private field used by the bus driver.
+ * @chipset_dev_no:		Private field used the bus driver.
+ * @state:			Used to indicate the current state of the
  *				device.
- * inst:			Unique GUID for this instance of the device.
- * name:			Name of the device.
- * pending_msg_hdr:		For private use by bus driver to respond to
+ * @inst:			Unique GUID for this instance of the device.
+ * @name:			Name of the device.
+ * @pending_msg_hdr:		For private use by bus driver to respond to
  *				hypervisor requests.
- * vbus_hdr_info:		A pointer to header info. Private use by bus
+ * @vbus_hdr_info:		A pointer to header info. Private use by bus
  *				driver.
- * partition_uuid:		Indicates client partion id. This should be the
+ * @partition_uuid:		Indicates client partion id. This should be the
  *				same across all visor_devices in the current
  *				guest. Private use by bus driver only.
  */
-- 
1.9.1

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

[PATCH 23/28] staging: unisys: visorbus: Remove notifier-related code from visorbus

[David Kershner]

From: David Binder <david.binder@unisys.com>

When this functionality was first implemented, visorchipset and visorbus
were separate drivers, which necessitated a registration mechanism for
them to communicate.  More-recently, visorchipset and visorbus were
combined into a single driver, and now exist as separate source files
within the same driver, known as 'visorbus'.  This eliminated the need
for a registration mechanism, but it has remained nevertheless until now.
For the sake of simplification, this registration mechanism is now being
removed.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <timothy.sell@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c    |  76 +++---------
 drivers/staging/unisys/visorbus/visorbus_private.h |  50 +++-----
 drivers/staging/unisys/visorbus/visorchipset.c     | 130 ++++-----------------
 3 files changed, 50 insertions(+), 206 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 2468264..c361077 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -119,32 +119,6 @@ struct bus_type visorbus_type = {
 static long long bus_count;	/* number of bus instances */
 					/* ever-increasing */
 
-static void chipset_bus_create(struct visor_device *bus_info);
-static void chipset_bus_destroy(struct visor_device *bus_info);
-static void chipset_device_create(struct visor_device *dev_info);
-static void chipset_device_destroy(struct visor_device *dev_info);
-static void chipset_device_pause(struct visor_device *dev_info);
-static void chipset_device_resume(struct visor_device *dev_info);
-
-/*
- * These functions are implemented herein, and are called by the chipset
- * driver to notify us about specific events.
- */
-static struct visorchipset_busdev_notifiers chipset_notifiers = {
-	.bus_create = chipset_bus_create,
-	.bus_destroy = chipset_bus_destroy,
-	.device_create = chipset_device_create,
-	.device_destroy = chipset_device_destroy,
-	.device_pause = chipset_device_pause,
-	.device_resume = chipset_device_resume,
-};
-
-/*
- * These functions are implemented in the chipset driver, and we call them
- * herein when we want to acknowledge a specific event.
- */
-static struct visorchipset_busdev_responders chipset_responders;
-
 /* filled in with info about parent chipset driver when we register with it */
 static struct ultra_vbus_deviceinfo chipset_driverinfo;
 /* filled in with info about this driver, wrt it servicing client busses */
@@ -1171,7 +1145,7 @@ remove_all_visor_devices(void)
 	}
 }
 
-static void
+void
 chipset_bus_create(struct visor_device *dev)
 {
 	int rc;
@@ -1188,19 +1162,17 @@ chipset_bus_create(struct visor_device *dev)
 		POSTCODE_LINUX_3(CHIPSET_INIT_SUCCESS_PC, bus_no,
 				 POSTCODE_SEVERITY_INFO);
 
-	if (chipset_responders.bus_create)
-		(*chipset_responders.bus_create) (dev, rc);
+	bus_create_response(dev, rc);
 }
 
-static void
+void
 chipset_bus_destroy(struct visor_device *dev)
 {
 	remove_bus_instance(dev);
-	if (chipset_responders.bus_destroy)
-		(*chipset_responders.bus_destroy)(dev, 0);
+	bus_destroy_response(dev, 0);
 }
 
-static void
+void
 chipset_device_create(struct visor_device *dev_info)
 {
 	int rc;
@@ -1211,8 +1183,7 @@ chipset_device_create(struct visor_device *dev_info)
 			 POSTCODE_SEVERITY_INFO);
 
 	rc = create_visor_device(dev_info);
-	if (chipset_responders.device_create)
-		chipset_responders.device_create(dev_info, rc);
+	device_create_response(dev_info, rc);
 
 	if (rc < 0)
 		POSTCODE_LINUX_4(DEVICE_CREATE_FAILURE_PC, dev_no, bus_no,
@@ -1222,13 +1193,12 @@ chipset_device_create(struct visor_device *dev_info)
 				 POSTCODE_SEVERITY_INFO);
 }
 
-static void
+void
 chipset_device_destroy(struct visor_device *dev_info)
 {
 	remove_visor_device(dev_info);
 
-	if (chipset_responders.device_destroy)
-		(*chipset_responders.device_destroy) (dev_info, 0);
+	device_destroy_response(dev_info, 0);
 }
 
 /**
@@ -1247,14 +1217,8 @@ pause_state_change_complete(struct visor_device *dev, int status)
 		return;
 
 	dev->pausing = false;
-	if (!chipset_responders.device_pause) /* this can never happen! */
-		return;
 
-	/* Notify the chipset driver that the pause is complete, which
-	 * will presumably want to send some sort of response to the
-	 * initiator.
-	 */
-	(*chipset_responders.device_pause) (dev, status);
+	visorchipset_device_pause_response(dev, status);
 }
 
 /**
@@ -1273,15 +1237,13 @@ resume_state_change_complete(struct visor_device *dev, int status)
 		return;
 
 	dev->resuming = false;
-	if (!chipset_responders.device_resume) /* this can never happen! */
-		return;
 
 	/*
 	 * Notify the chipset driver that the resume is complete,
 	 * which will presumably want to send some sort of response to
 	 * the initiator.
 	 */
-	(*chipset_responders.device_resume) (dev, status);
+	device_resume_response(dev, status);
 }
 
 /**
@@ -1303,9 +1265,9 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 	void (*notify_func)(struct visor_device *dev, int response) = NULL;
 
 	if (is_pause)
-		notify_func = chipset_responders.device_pause;
+		notify_func = visorchipset_device_pause_response;
 	else
-		notify_func = chipset_responders.device_resume;
+		notify_func = device_resume_response;
 	if (!notify_func)
 		return;
 
@@ -1369,7 +1331,7 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
  * that device.  Success/failure result is returned asynchronously
  * via a callback function; see pause_state_change_complete().
  */
-static void
+void
 chipset_device_pause(struct visor_device *dev_info)
 {
 	initiate_chipset_device_pause_resume(dev_info, true);
@@ -1383,7 +1345,7 @@ chipset_device_pause(struct visor_device *dev_info)
  * that device.  Success/failure result is returned asynchronously
  * via a callback function; see resume_state_change_complete().
  */
-static void
+void
 chipset_device_resume(struct visor_device *dev_info)
 {
 	initiate_chipset_device_pause_resume(dev_info, false);
@@ -1405,12 +1367,9 @@ visorbus_init(void)
 		goto error;
 	}
 
-	/* This enables us to receive notifications when devices appear for
-	 * which this service partition is to be a server for.
-	 */
-	visorchipset_register_busdev(&chipset_notifiers,
-				     &chipset_responders,
-				     &chipset_driverinfo);
+	bus_device_info_init(&chipset_driverinfo,
+			     "chipset", "visorchipset",
+			     VERSION, NULL);
 
 	return 0;
 
@@ -1424,7 +1383,6 @@ visorbus_exit(void)
 {
 	struct list_head *listentry, *listtmp;
 
-	visorchipset_register_busdev(NULL, NULL, NULL);
 	remove_all_visor_devices();
 
 	list_for_each_safe(listentry, listtmp, &list_all_bus_instances) {
diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/staging/unisys/visorbus/visorbus_private.h
index 31b5ca9..3b5a8f2 100644
--- a/drivers/staging/unisys/visorbus/visorbus_private.h
+++ b/drivers/staging/unisys/visorbus/visorbus_private.h
@@ -23,44 +23,20 @@
 #include "vbusdeviceinfo.h"
 #include "vbushelper.h"
 
-/*  These functions will be called from within visorchipset when certain
- *  events happen.  (The implementation of these functions is outside of
- *  visorchipset.)
- */
-struct visorchipset_busdev_notifiers {
-	void (*bus_create)(struct visor_device *bus_info);
-	void (*bus_destroy)(struct visor_device *bus_info);
-	void (*device_create)(struct visor_device *bus_info);
-	void (*device_destroy)(struct visor_device *bus_info);
-	void (*device_pause)(struct visor_device *bus_info);
-	void (*device_resume)(struct visor_device *bus_info);
-};
-
-/*  These functions live inside visorchipset, and will be called to indicate
- *  responses to specific events (by code outside of visorchipset).
- *  For now, the value for each response is simply either:
- *       0 = it worked
- *      -1 = it failed
- */
-struct visorchipset_busdev_responders {
-	void (*bus_create)(struct visor_device *p, int response);
-	void (*bus_destroy)(struct visor_device *p, int response);
-	void (*device_create)(struct visor_device *p, int response);
-	void (*device_destroy)(struct visor_device *p, int response);
-	void (*device_pause)(struct visor_device *p, int response);
-	void (*device_resume)(struct visor_device *p, int response);
-};
+void chipset_bus_create(struct visor_device *bus_info);
+void chipset_bus_destroy(struct visor_device *bus_info);
+void chipset_device_create(struct visor_device *dev_info);
+void chipset_device_destroy(struct visor_device *dev_info);
+void chipset_device_pause(struct visor_device *dev_info);
+void chipset_device_resume(struct visor_device *dev_info);
 
-/** Register functions (in the bus driver) to get called by visorchipset
- *  whenever a bus or device appears for which this guest is to be the
- *  client for.  visorchipset will fill in <responders>, to indicate
- *  functions the bus driver should call to indicate message responses.
- */
-void
-visorchipset_register_busdev(
-			struct visorchipset_busdev_notifiers *notifiers,
-			struct visorchipset_busdev_responders *responders,
-			struct ultra_vbus_deviceinfo *driver_info);
+void bus_create_response(struct visor_device *p, int response);
+void bus_destroy_response(struct visor_device *p, int response);
+void device_create_response(struct visor_device *p, int response);
+void device_destroy_response(struct visor_device *p, int response);
+void device_resume_response(struct visor_device *p, int response);
+void visorchipset_device_pause_response(struct visor_device *p,
+					int response);
 
 /* visorbus init and exit functions */
 int visorbus_init(void);
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index f8e1fa5..ea548df 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -87,7 +87,6 @@ visorchipset_release(struct inode *inode, struct file *file)
 static unsigned long poll_jiffies = POLLJIFFIES_CONTROLVMCHANNEL_FAST;
 /* when we got our last controlvm message */
 static unsigned long most_recent_message_jiffies;
-static int visorbusregistered;
 
 struct parser_context {
 	unsigned long allocbytes;
@@ -99,7 +98,6 @@ struct parser_context {
 };
 
 static struct delayed_work periodic_controlvm_work;
-static DEFINE_SEMAPHORE(notifier_lock);
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
@@ -212,26 +210,6 @@ static LIST_HEAD(parahotplug_request_list);
 static DEFINE_SPINLOCK(parahotplug_request_list_lock);	/* lock for above */
 static void parahotplug_process_list(void);
 
-static struct visorchipset_busdev_notifiers busdev_notifiers;
-
-static void bus_create_response(struct visor_device *p, int response);
-static void bus_destroy_response(struct visor_device *p, int response);
-static void device_create_response(struct visor_device *p, int response);
-static void device_destroy_response(struct visor_device *p, int response);
-static void device_resume_response(struct visor_device *p, int response);
-
-static void visorchipset_device_pause_response(struct visor_device *p,
-					       int response);
-
-static struct visorchipset_busdev_responders busdev_responders = {
-	.bus_create = bus_create_response,
-	.bus_destroy = bus_destroy_response,
-	.device_create = device_create_response,
-	.device_destroy = device_destroy_response,
-	.device_pause = visorchipset_device_pause_response,
-	.device_resume = device_resume_response,
-};
-
 /* info for /dev/visorchipset */
 static dev_t major_dev = -1; /*< indicates major num for device */
 
@@ -691,30 +669,6 @@ struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
 	return vdev;
 }
 
-void
-visorchipset_register_busdev(
-			struct visorchipset_busdev_notifiers *notifiers,
-			struct visorchipset_busdev_responders *responders,
-			struct ultra_vbus_deviceinfo *driver_info)
-{
-	down(&notifier_lock);
-	if (!notifiers) {
-		memset(&busdev_notifiers, 0,
-		       sizeof(busdev_notifiers));
-		visorbusregistered = 0;	/* clear flag */
-	} else {
-		busdev_notifiers = *notifiers;
-		visorbusregistered = 1;	/* set flag */
-	}
-	if (responders)
-		*responders = busdev_responders;
-	if (driver_info)
-		bus_device_info_init(driver_info, "chipset", "visorchipset",
-				     VERSION, NULL);
-
-	up(&notifier_lock);
-}
-
 static void
 chipset_init(struct controlvm_message *inmsg)
 {
@@ -927,22 +881,20 @@ bus_epilog(struct visor_device *bus_info,
 {
 	struct controlvm_message_header *pmsg_hdr = NULL;
 
-	down(&notifier_lock);
-
 	if (!bus_info) {
 		/*
 		 * relying on a valid passed in response code
 		 * be lazy and re-use msg_hdr for this failure, is this ok??
 		 */
 		pmsg_hdr = msg_hdr;
-		goto out_respond_and_unlock;
+		goto out_respond;
 	}
 
 	if (bus_info->pending_msg_hdr) {
 		/* only non-NULL if dev is still waiting on a response */
 		response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
 		pmsg_hdr = bus_info->pending_msg_hdr;
-		goto out_respond_and_unlock;
+		goto out_respond;
 	}
 
 	if (need_response) {
@@ -951,7 +903,7 @@ bus_epilog(struct visor_device *bus_info,
 			POSTCODE_LINUX_4(MALLOC_FAILURE_PC, cmd,
 					 bus_info->chipset_bus_no,
 					 POSTCODE_SEVERITY_ERR);
-			goto out_unlock;
+			return;
 		}
 
 		memcpy(pmsg_hdr, msg_hdr,
@@ -962,25 +914,16 @@ bus_epilog(struct visor_device *bus_info,
 	if (response == CONTROLVM_RESP_SUCCESS) {
 		switch (cmd) {
 		case CONTROLVM_BUS_CREATE:
-			if (busdev_notifiers.bus_create) {
-				(*busdev_notifiers.bus_create) (bus_info);
-				goto out_unlock;
-			}
+			chipset_bus_create(bus_info);
 			break;
 		case CONTROLVM_BUS_DESTROY:
-			if (busdev_notifiers.bus_destroy) {
-				(*busdev_notifiers.bus_destroy) (bus_info);
-				goto out_unlock;
-			}
+			chipset_bus_destroy(bus_info);
 			break;
 		}
 	}
 
-out_respond_and_unlock:
+out_respond:
 	bus_responder(cmd, pmsg_hdr, response);
-
-out_unlock:
-	up(&notifier_lock);
 }
 
 static void
@@ -989,33 +932,29 @@ device_epilog(struct visor_device *dev_info,
 	      struct controlvm_message_header *msg_hdr, int response,
 	      bool need_response, bool for_visorbus)
 {
-	struct visorchipset_busdev_notifiers *notifiers;
 	struct controlvm_message_header *pmsg_hdr = NULL;
 
-	notifiers = &busdev_notifiers;
-
-	down(&notifier_lock);
 	if (!dev_info) {
 		/*
 		 * relying on a valid passed in response code
 		 * be lazy and re-use msg_hdr for this failure, is this ok??
 		 */
 		pmsg_hdr = msg_hdr;
-		goto out_respond_and_unlock;
+		goto out_respond;
 	}
 
 	if (dev_info->pending_msg_hdr) {
 		/* only non-NULL if dev is still waiting on a response */
 		response = -CONTROLVM_RESP_ERROR_MESSAGE_ID_INVALID_FOR_CLIENT;
 		pmsg_hdr = dev_info->pending_msg_hdr;
-		goto out_respond_and_unlock;
+		goto out_respond;
 	}
 
 	if (need_response) {
 		pmsg_hdr = kzalloc(sizeof(*pmsg_hdr), GFP_KERNEL);
 		if (!pmsg_hdr) {
 			response = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
-			goto out_respond_and_unlock;
+			goto out_respond;
 		}
 
 		memcpy(pmsg_hdr, msg_hdr,
@@ -1026,20 +965,14 @@ device_epilog(struct visor_device *dev_info,
 	if (response >= 0) {
 		switch (cmd) {
 		case CONTROLVM_DEVICE_CREATE:
-			if (notifiers->device_create) {
-				(*notifiers->device_create) (dev_info);
-				goto out_unlock;
-			}
+			chipset_device_create(dev_info);
 			break;
 		case CONTROLVM_DEVICE_CHANGESTATE:
 			/* ServerReady / ServerRunning / SegmentStateRunning */
 			if (state.alive == segment_state_running.alive &&
 			    state.operating ==
 				segment_state_running.operating) {
-				if (notifiers->device_resume) {
-					(*notifiers->device_resume) (dev_info);
-					goto out_unlock;
-				}
+				chipset_device_resume(dev_info);
 			}
 			/* ServerNotReady / ServerLost / SegmentStateStandby */
 			else if (state.alive == segment_state_standby.alive &&
@@ -1049,26 +982,17 @@ device_epilog(struct visor_device *dev_info,
 				 * technically this is standby case
 				 * where server is lost
 				 */
-				if (notifiers->device_pause) {
-					(*notifiers->device_pause) (dev_info);
-					goto out_unlock;
-				}
+				chipset_device_pause(dev_info);
 			}
 			break;
 		case CONTROLVM_DEVICE_DESTROY:
-			if (notifiers->device_destroy) {
-				(*notifiers->device_destroy) (dev_info);
-				goto out_unlock;
-			}
+			chipset_device_destroy(dev_info);
 			break;
 		}
 	}
 
-out_respond_and_unlock:
+out_respond:
 	device_responder(cmd, pmsg_hdr, response);
-
-out_unlock:
-	up(&notifier_lock);
 }
 
 static void
@@ -1866,10 +1790,6 @@ controlvm_periodic_work(struct work_struct *work)
 	bool got_command = false;
 	bool handle_command_failed = false;
 
-	/* make sure visorbus server is registered for controlvm callbacks */
-	if (visorchipset_visorbusregwait && !visorbusregistered)
-		goto cleanup;
-
 	while (visorchannel_signalremove(controlvm_channel,
 					 CONTROLVM_QUEUE_RESPONSE,
 					 &inmsg))
@@ -1913,8 +1833,6 @@ controlvm_periodic_work(struct work_struct *work)
 	/* parahotplug_worker */
 	parahotplug_process_list();
 
-cleanup:
-
 	if (time_after(jiffies,
 		       most_recent_message_jiffies + (HZ * MIN_IDLE_SECONDS))) {
 		/*
@@ -1941,13 +1859,6 @@ setup_crash_devices_work_queue(struct work_struct *work)
 	u32 local_crash_msg_offset;
 	u16 local_crash_msg_count;
 
-	/* make sure visorbus is registered for controlvm callbacks */
-	if (visorchipset_visorbusregwait && !visorbusregistered) {
-		poll_jiffies = POLLJIFFIES_CONTROLVMCHANNEL_SLOW;
-		schedule_delayed_work(&periodic_controlvm_work, poll_jiffies);
-		return;
-	}
-
 	POSTCODE_LINUX_2(CRASH_DEV_ENTRY_PC, POSTCODE_SEVERITY_INFO);
 
 	/* send init chipset msg */
@@ -2025,7 +1936,7 @@ setup_crash_devices_work_queue(struct work_struct *work)
 	POSTCODE_LINUX_2(CRASH_DEV_EXIT_PC, POSTCODE_SEVERITY_INFO);
 }
 
-static void
+void
 bus_create_response(struct visor_device *bus_info, int response)
 {
 	if (response >= 0)
@@ -2038,7 +1949,7 @@ bus_create_response(struct visor_device *bus_info, int response)
 	bus_info->pending_msg_hdr = NULL;
 }
 
-static void
+void
 bus_destroy_response(struct visor_device *bus_info, int response)
 {
 	bus_responder(CONTROLVM_BUS_DESTROY, bus_info->pending_msg_hdr,
@@ -2048,7 +1959,7 @@ bus_destroy_response(struct visor_device *bus_info, int response)
 	bus_info->pending_msg_hdr = NULL;
 }
 
-static void
+void
 device_create_response(struct visor_device *dev_info, int response)
 {
 	if (response >= 0)
@@ -2061,7 +1972,7 @@ device_create_response(struct visor_device *dev_info, int response)
 	dev_info->pending_msg_hdr = NULL;
 }
 
-static void
+void
 device_destroy_response(struct visor_device *dev_info, int response)
 {
 	device_responder(CONTROLVM_DEVICE_DESTROY, dev_info->pending_msg_hdr,
@@ -2071,7 +1982,7 @@ device_destroy_response(struct visor_device *dev_info, int response)
 	dev_info->pending_msg_hdr = NULL;
 }
 
-static void
+void
 visorchipset_device_pause_response(struct visor_device *dev_info,
 				   int response)
 {
@@ -2083,7 +1994,7 @@ visorchipset_device_pause_response(struct visor_device *dev_info,
 	dev_info->pending_msg_hdr = NULL;
 }
 
-static void
+void
 device_resume_response(struct visor_device *dev_info, int response)
 {
 	device_changestate_responder(CONTROLVM_DEVICE_CHANGESTATE,
@@ -2290,7 +2201,6 @@ visorchipset_init(struct acpi_device *acpi_device)
 	if (!addr)
 		goto error;
 
-	memset(&busdev_notifiers, 0, sizeof(busdev_notifiers));
 	memset(&controlvm_payload_info, 0, sizeof(controlvm_payload_info));
 
 	controlvm_channel = visorchannel_create_with_lock(addr, 0,
-- 
1.9.1

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

[PATCH 24/28] staging: unisys: visorbus: Rename function to follow existing convention

[David Kershner]

From: David Binder <david.binder@unisys.com>

Renames visorchipset_device_pause_response to device_pause_response,
thereby following the convention that other responder functions follow.

Signed-off-by: David Binder <david.binder@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c    | 4 ++--
 drivers/staging/unisys/visorbus/visorbus_private.h | 3 +--
 drivers/staging/unisys/visorbus/visorchipset.c     | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index c361077..293532f 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -1218,7 +1218,7 @@ pause_state_change_complete(struct visor_device *dev, int status)
 
 	dev->pausing = false;
 
-	visorchipset_device_pause_response(dev, status);
+	device_pause_response(dev, status);
 }
 
 /**
@@ -1265,7 +1265,7 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 	void (*notify_func)(struct visor_device *dev, int response) = NULL;
 
 	if (is_pause)
-		notify_func = visorchipset_device_pause_response;
+		notify_func = device_pause_response;
 	else
 		notify_func = device_resume_response;
 	if (!notify_func)
diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/staging/unisys/visorbus/visorbus_private.h
index 3b5a8f2..a7b1d4f 100644
--- a/drivers/staging/unisys/visorbus/visorbus_private.h
+++ b/drivers/staging/unisys/visorbus/visorbus_private.h
@@ -35,8 +35,7 @@ void bus_destroy_response(struct visor_device *p, int response);
 void device_create_response(struct visor_device *p, int response);
 void device_destroy_response(struct visor_device *p, int response);
 void device_resume_response(struct visor_device *p, int response);
-void visorchipset_device_pause_response(struct visor_device *p,
-					int response);
+void device_pause_response(struct visor_device *p, int response);
 
 /* visorbus init and exit functions */
 int visorbus_init(void);
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index ea548df..0b4a138 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1983,8 +1983,8 @@ device_destroy_response(struct visor_device *dev_info, int response)
 }
 
 void
-visorchipset_device_pause_response(struct visor_device *dev_info,
-				   int response)
+device_pause_response(struct visor_device *dev_info,
+		      int response)
 {
 	device_changestate_responder(CONTROLVM_DEVICE_CHANGESTATE,
 				     dev_info, response,
-- 
1.9.1

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

[PATCH 25/28] staging: unisys: visorbus: fix visorbus_private.h comments

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

This patch ONLY touches comment lines, i.e., NO executable code is
affected.

Comments were fixed in visorbus_private.h:
* Minor typos were corrected.
* Useless comments were removed.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_private.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/staging/unisys/visorbus/visorbus_private.h
index a7b1d4f..3f6ad52 100644
--- a/drivers/staging/unisys/visorbus/visorbus_private.h
+++ b/drivers/staging/unisys/visorbus/visorbus_private.h
@@ -37,16 +37,11 @@ void device_destroy_response(struct visor_device *p, int response);
 void device_resume_response(struct visor_device *p, int response);
 void device_pause_response(struct visor_device *p, int response);
 
-/* visorbus init and exit functions */
 int visorbus_init(void);
 void visorbus_exit(void);
 
-/* Visorchannel access functions */
+/* visorchannel access functions */
 
-/* Note that for visorchannel_create()
- * <channel_bytes> and <guid> arguments may be 0 if we are a channel CLIENT.
- * In this case, the values can simply be read from the channel header.
- */
 struct visorchannel *visorchannel_create(u64 physaddr,
 					 unsigned long channel_bytes,
 					 gfp_t gfp, uuid_le guid);
-- 
1.9.1

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

[PATCH 26/28] staging: unisys: Move vbushelper.h to visorbus directory

[David Kershner]

Only visorbus needs this header file so move it to visorbus
directory.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
---
 drivers/staging/unisys/{include => visorbus}/vbushelper.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename drivers/staging/unisys/{include => visorbus}/vbushelper.h (100%)

diff --git a/drivers/staging/unisys/include/vbushelper.h b/drivers/staging/unisys/visorbus/vbushelper.h
similarity index 100%
rename from drivers/staging/unisys/include/vbushelper.h
rename to drivers/staging/unisys/visorbus/vbushelper.h
-- 
1.9.1

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

[PATCH 27/28] staging: unisys: visorinput: ensure proper locking wrt creation & ints

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

Ensure we properly lock between visorinput_channel_interrupt(),
visorinput_open(), and devdata_create().  We now guarantee that:

* interrupts will be disabled and remain disabled during device creation,
  by setting 'paused = true' across device creation

* we canNOT get into visorinput_open() until the device structure is
  totally initialized, by delaying the input_register_device() until the
  end of device initialization

We also now ensure that lock_visor_dev is held across updates of devdata
state, to ensure state consistency.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorinput/visorinput.c | 74 +++++++++++++++++---------
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index f633985..c13e698 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -277,16 +277,16 @@ out_unlock:
 }
 
 /*
- * register_client_keyboard() initializes and returns a Linux input node that
+ * setup_client_keyboard() initializes and returns a Linux input node that
  * we can use to deliver keyboard inputs to Linux.  We of course do this when
  * we see keyboard inputs coming in on a keyboard channel.
  */
 static struct input_dev *
-register_client_keyboard(void *devdata,  /* opaque on purpose */
-			 unsigned char *keycode_table)
+setup_client_keyboard(void *devdata,  /* opaque on purpose */
+		      unsigned char *keycode_table)
 
 {
-	int i, error;
+	int i;
 	struct input_dev *visorinput_dev;
 
 	visorinput_dev = input_allocate_device();
@@ -320,18 +320,12 @@ register_client_keyboard(void *devdata,  /* opaque on purpose */
 	visorinput_dev->close = visorinput_close;
 	input_set_drvdata(visorinput_dev, devdata); /* pre input_register! */
 
-	error = input_register_device(visorinput_dev);
-	if (error) {
-		input_free_device(visorinput_dev);
-		return NULL;
-	}
 	return visorinput_dev;
 }
 
 static struct input_dev *
-register_client_mouse(void *devdata /* opaque on purpose */)
+setup_client_mouse(void *devdata /* opaque on purpose */)
 {
-	int error;
 	struct input_dev *visorinput_dev = NULL;
 	int xres, yres;
 	struct fb_info *fb0;
@@ -366,13 +360,6 @@ register_client_mouse(void *devdata /* opaque on purpose */)
 	visorinput_dev->open = visorinput_open;
 	visorinput_dev->close = visorinput_close;
 	input_set_drvdata(visorinput_dev, devdata); /* pre input_register! */
-
-	error = input_register_device(visorinput_dev);
-	if (error) {
-		input_free_device(visorinput_dev);
-		return NULL;
-	}
-
 	input_set_capability(visorinput_dev, EV_REL, REL_WHEEL);
 
 	return visorinput_dev;
@@ -390,9 +377,19 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 	devdata = kzalloc(sizeof(*devdata) + extra_bytes, GFP_KERNEL);
 	if (!devdata)
 		return NULL;
+	init_rwsem(&devdata->lock_visor_dev);
+	down_write(&devdata->lock_visor_dev);
 	devdata->dev = dev;
 
 	/*
+	 * visorinput_open() can be called as soon as input_register_device()
+	 * happens, and that will enable channel interrupts.  Setting paused
+	 * prevents us from getting into visorinput_channel_interrupt() prior
+	 * to the device structure being totally initialized.
+	 */
+	devdata->paused = true;
+
+	/*
 	 * This is an input device in a client guest partition,
 	 * so we need to create whatever input nodes are necessary to
 	 * deliver our inputs to the guest OS.
@@ -404,23 +401,49 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 		       KEYCODE_TABLE_BYTES);
 		memcpy(devdata->keycode_table + KEYCODE_TABLE_BYTES,
 		       visorkbd_ext_keycode, KEYCODE_TABLE_BYTES);
-		devdata->visorinput_dev = register_client_keyboard
+		devdata->visorinput_dev = setup_client_keyboard
 			(devdata, devdata->keycode_table);
 		if (!devdata->visorinput_dev)
 			goto cleanups_register;
 		break;
 	case visorinput_mouse:
-		devdata->visorinput_dev = register_client_mouse(devdata);
+		devdata->visorinput_dev = setup_client_mouse(devdata);
 		if (!devdata->visorinput_dev)
 			goto cleanups_register;
 		break;
 	}
 
-	init_rwsem(&devdata->lock_visor_dev);
+	dev_set_drvdata(&dev->device, devdata);
+	up_write(&devdata->lock_visor_dev);
+
+	/*
+	 * Device struct is completely set up now, with the exception of
+	 * visorinput_dev being registered.
+	 * We need to unlock before we register the device, because this
+	 * can cause an on-stack call of visorinput_open(), which would
+	 * deadlock if we had the lock.
+	 */
+	if (input_register_device(devdata->visorinput_dev)) {
+		input_free_device(devdata->visorinput_dev);
+		goto err_kfree_devdata;
+	}
+
+	down_write(&devdata->lock_visor_dev);
+	/*
+	 * Establish calls to visorinput_channel_interrupt() if that is
+	 * the desired state that we've kept track of in interrupts_enabled
+	 * while the device was being created.
+	 */
+	devdata->paused = false;
+	if (devdata->interrupts_enabled)
+		visorbus_enable_channel_interrupts(dev);
+	up_write(&devdata->lock_visor_dev);
 
 	return devdata;
 
 cleanups_register:
+	up_write(&devdata->lock_visor_dev);
+err_kfree_devdata:
 	kfree(devdata);
 	return NULL;
 }
@@ -428,7 +451,6 @@ cleanups_register:
 static int
 visorinput_probe(struct visor_device *dev)
 {
-	struct visorinput_devdata *devdata = NULL;
 	uuid_le guid;
 	enum visorinput_device_type devtype;
 
@@ -439,10 +461,9 @@ visorinput_probe(struct visor_device *dev)
 		devtype = visorinput_keyboard;
 	else
 		return -ENODEV;
-	devdata = devdata_create(dev, devtype);
-	if (!devdata)
+	visorbus_disable_channel_interrupts(dev);
+	if (!devdata_create(dev, devtype))
 		return -ENOMEM;
-	dev_set_drvdata(&dev->device, devdata);
 	return 0;
 }
 
@@ -461,6 +482,7 @@ visorinput_remove(struct visor_device *dev)
 	if (!devdata)
 		return;
 
+	down_write(&devdata->lock_visor_dev);
 	visorbus_disable_channel_interrupts(dev);
 
 	/*
@@ -469,6 +491,8 @@ visorinput_remove(struct visor_device *dev)
 	 */
 
 	dev_set_drvdata(&dev->device, NULL);
+	up_write(&devdata->lock_visor_dev);
+
 	unregister_client_input(devdata->visorinput_dev);
 	kfree(devdata);
 }
-- 
1.9.1

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

[PATCH 28/28] staging: unisys: visorinput: make lock_visor_dev a mutex

[David Kershner]

From: Tim Sell <Timothy.Sell@unisys.com>

Since lock_visor_dev is a binary semaphore it makes more sense to
use a mutex lock.

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
---
 drivers/staging/unisys/visorinput/visorinput.c | 34 +++++++++++++-------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index c13e698..2aff945 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -63,7 +63,7 @@ enum visorinput_device_type {
  */
 struct visorinput_devdata {
 	struct visor_device *dev;
-	struct rw_semaphore lock_visor_dev; /* lock for dev */
+	struct mutex lock_visor_dev; /* lock for dev */
 	struct input_dev *visorinput_dev;
 	bool paused;
 	bool interrupts_enabled;
@@ -236,14 +236,14 @@ static int visorinput_open(struct input_dev *visorinput_dev)
 	 * interrupts should be enabled so when we resume, interrupts
 	 * will really be enabled.
 	 */
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	devdata->interrupts_enabled = true;
 	if (devdata->paused)
 		goto out_unlock;
 	visorbus_enable_channel_interrupts(devdata->dev);
 
 out_unlock:
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 	return 0;
 }
 
@@ -266,14 +266,14 @@ static void visorinput_close(struct input_dev *visorinput_dev)
 	 * not re-enable them.
 	 */
 
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	devdata->interrupts_enabled = false;
 	if (devdata->paused)
 		goto out_unlock;
 	visorbus_disable_channel_interrupts(devdata->dev);
 
 out_unlock:
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 }
 
 /*
@@ -377,8 +377,8 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 	devdata = kzalloc(sizeof(*devdata) + extra_bytes, GFP_KERNEL);
 	if (!devdata)
 		return NULL;
-	init_rwsem(&devdata->lock_visor_dev);
-	down_write(&devdata->lock_visor_dev);
+	mutex_init(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	devdata->dev = dev;
 
 	/*
@@ -414,7 +414,7 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 	}
 
 	dev_set_drvdata(&dev->device, devdata);
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 
 	/*
 	 * Device struct is completely set up now, with the exception of
@@ -428,7 +428,7 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 		goto err_kfree_devdata;
 	}
 
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	/*
 	 * Establish calls to visorinput_channel_interrupt() if that is
 	 * the desired state that we've kept track of in interrupts_enabled
@@ -437,12 +437,12 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 	devdata->paused = false;
 	if (devdata->interrupts_enabled)
 		visorbus_enable_channel_interrupts(dev);
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 
 	return devdata;
 
 cleanups_register:
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 err_kfree_devdata:
 	kfree(devdata);
 	return NULL;
@@ -482,7 +482,7 @@ visorinput_remove(struct visor_device *dev)
 	if (!devdata)
 		return;
 
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	visorbus_disable_channel_interrupts(dev);
 
 	/*
@@ -491,7 +491,7 @@ visorinput_remove(struct visor_device *dev)
 	 */
 
 	dev_set_drvdata(&dev->device, NULL);
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 
 	unregister_client_input(devdata->visorinput_dev);
 	kfree(devdata);
@@ -671,7 +671,7 @@ visorinput_pause(struct visor_device *dev,
 		goto out;
 	}
 
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	if (devdata->paused) {
 		rc = -EBUSY;
 		goto out_locked;
@@ -688,7 +688,7 @@ visorinput_pause(struct visor_device *dev,
 	complete_func(dev, 0);
 	rc = 0;
 out_locked:
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 out:
 	return rc;
 }
@@ -704,7 +704,7 @@ visorinput_resume(struct visor_device *dev,
 		rc = -ENODEV;
 		goto out;
 	}
-	down_write(&devdata->lock_visor_dev);
+	mutex_lock(&devdata->lock_visor_dev);
 	if (!devdata->paused) {
 		rc = -EBUSY;
 		goto out_locked;
@@ -722,7 +722,7 @@ visorinput_resume(struct visor_device *dev,
 
 	rc = 0;
 out_locked:
-	up_write(&devdata->lock_visor_dev);
+	mutex_unlock(&devdata->lock_visor_dev);
 out:
 	return rc;
 }
-- 
1.9.1

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