All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] staging: unisys: add channel interrupt support
@ 2015-11-17 14:57 Benjamin Romer
  2015-11-17 14:57 ` [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work Benjamin Romer
                   ` (14 more replies)
  0 siblings, 15 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:57 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

This patch series adds a centralized infrastructure and device support
for channel interrupts sent to s-Par virtual devices. With these changes,
the visorhba device is ~80% faster than with only polling, and visornic
receives a speedup of over 3500% (from ~9Mb/s to between 360Mb/s and
390Mb/s).

David Kershner (13):
  staging: unisys: Change poll rate to 2 ms for work
  staging: unisys: set client state
  staging: unisys: visorhba: Convert visorhba to use visorbus channel
    interrupts.
  staging: unisys: Convert visornic to use visorbus channel interrupt
    code
  staging: unisys: Only process up to budget amount of responses
  staging: unisys: Add support to update Features bits in channel queues
  staging: unisys: Add channel feature access functions
  staging: unisys: Re-enable interrupts after we have done the work
  staging: unisys: Capture data from device create to register
    interrupt.
  staging: unisys: Don't go into POLLING mode in visornic_probe.
  staging: unisys: Don't set polling mode in visorhba_probe
  staging: unisys: Remove semaphores around channel interrupts.
  staging: unisys: Allow for unregistering of interrupts.

Tim Sell (1):
  staging: unisys: visorinput: use spinlock for channel_interrupt()
    locking

 drivers/staging/unisys/include/visorbus.h       |  13 ++
 drivers/staging/unisys/visorbus/visorbus_main.c | 259 +++++++++++++++++++++++-
 drivers/staging/unisys/visorbus/visorchannel.c  |  47 +++++
 drivers/staging/unisys/visorbus/visorchipset.c  |  17 +-
 drivers/staging/unisys/visorhba/visorhba_main.c | 138 ++++++-------
 drivers/staging/unisys/visorinput/visorinput.c  |  31 ++-
 drivers/staging/unisys/visornic/visornic_main.c |  74 +++----
 7 files changed, 437 insertions(+), 142 deletions(-)

-- 
2.5.0

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

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

* [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
@ 2015-11-17 14:57 ` Benjamin Romer
  2015-11-17 22:19   ` Greg KH
  2015-11-17 14:58 ` [PATCH 02/14] staging: unisys: set client state Benjamin Romer
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:57 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Use ms_to_jiffies for the periodic work queue instead of raw jiffies.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index a272b48..1fa6e5b 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -34,8 +34,8 @@ static int visorbus_debugref;
 #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024)
 
 #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
-#define POLLJIFFIES_TESTWORK         100
-#define POLLJIFFIES_NORMALCHANNEL     10
+#define POLLJIFFIES_TESTWORK         msecs_to_jiffies(100)
+#define POLLJIFFIES_NORMALCHANNEL     msecs_to_jiffies(2)
 
 static int busreg_rc = -ENODEV; /* stores the result from bus registration */
 
-- 
2.5.0

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

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

* [PATCH 02/14] staging: unisys: set client state
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
  2015-11-17 14:57 ` [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 03/14] staging: unisys: visorhba: Convert visorhba to use visorbus channel interrupts Benjamin Romer
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Every channel has an s-Par channel state associated with it. This patch
correctly sets of the channels.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 32 ++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 1fa6e5b..63e7863 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -747,6 +747,23 @@ dev_stop_periodic_work(struct visor_device *dev)
 		put_device(&dev->device);
 }
 
+int visorbus_set_channel_state(struct visor_device *dev, u32 cli_state)
+{
+	int channel_offset = 0, err = 0;
+
+	channel_offset = offsetof(struct channel_header, cli_state_os);
+
+	err = visorbus_write_channel(dev, channel_offset, &cli_state, 4);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to set client_state_os from chan (%d)\n",
+			__func__, err);
+		return err;
+	}
+
+	return err;
+}
+
 /** 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
@@ -769,6 +786,7 @@ visordriver_probe_device(struct device *xdev)
 	 */
 	wmb();
 	get_device(&dev->device);
+	visorbus_set_channel_state(dev, CHANNELCLI_OWNED);
 	if (!drv->probe) {
 		up(&dev->visordriver_callback_lock);
 		rc = -1;
@@ -782,8 +800,10 @@ visordriver_probe_device(struct device *xdev)
 	up(&dev->visordriver_callback_lock);
 	rc = 0;
 away:
-	if (rc != 0)
+	if (rc != 0) {
+		visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
 		put_device(&dev->device);
+	}
 	return rc;
 }
 
@@ -814,6 +834,7 @@ visordriver_remove_device(struct device *xdev)
 	dev_stop_periodic_work(dev);
 	devmajorminor_remove_all_files(dev);
 
+	visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
 	put_device(&dev->device);
 
 	return 0;
@@ -1036,6 +1057,8 @@ create_visor_device(struct visor_device *dev)
 	}
 
 	list_add_tail(&dev->list_all, &list_all_device_instances);
+
+	visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
 	return 0;
 
 away_register:
@@ -1388,6 +1411,9 @@ resume_state_change_complete(struct visor_device *dev, int status)
 	if (!chipset_responders.device_resume) /* this can never happen! */
 		return;
 
+	if (status < 0)
+		visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
+
 	/* Notify the chipset driver that the resume is complete,
 	 * which will presumably want to send some sort of response to
 	 * the initiator. */
@@ -1409,6 +1435,7 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 		notify_func = chipset_responders.device_pause;
 	else
 		notify_func = chipset_responders.device_resume;
+
 	if (!notify_func)
 		goto away;
 
@@ -1443,6 +1470,7 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 			goto away;
 
 		dev->resuming = true;
+		visorbus_set_channel_state(dev, CHANNELCLI_OWNED);
 		x = drv->resume(dev, resume_state_change_complete);
 	}
 	if (x < 0) {
@@ -1455,6 +1483,8 @@ initiate_chipset_device_pause_resume(struct visor_device *dev, bool is_pause)
 	rc = 0;
 away:
 	if (rc < 0) {
+		if (!is_pause)
+			visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
 		if (notify_func)
 			(*notify_func)(dev, rc);
 	}
-- 
2.5.0

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

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

* [PATCH 03/14] staging: unisys: visorhba: Convert visorhba to use visorbus channel interrupts.
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
  2015-11-17 14:57 ` [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work Benjamin Romer
  2015-11-17 14:58 ` [PATCH 02/14] staging: unisys: set client state Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 04/14] staging: unisys: Convert visornic to use visorbus channel interrupt code Benjamin Romer
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, David Kershner, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Use a tasklet to process the response queue from the IO Service
Partition instead of using a thread. Register with visorbus to have it
issue either "soft" interrupts or "s-Par" interrupts depending on
registration values from the create message.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorhba/visorhba_main.c | 114 +++++++++++-------------
 1 file changed, 52 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index c119f20..70bc878 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -16,6 +16,7 @@
 #include <linux/debugfs.h>
 #include <linux/skbuff.h>
 #include <linux/kthread.h>
+#include <linux/interrupt.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_cmnd.h>
@@ -66,23 +67,6 @@ static struct visor_channeltype_descriptor visorhba_channel_types[] = {
 	{ NULL_UUID_LE, NULL }
 };
 
-/* This is used to tell the visor bus driver which types of visor devices
- * we support, and what functions to call when a visor device that we support
- * is attached or removed.
- */
-static struct visor_driver visorhba_driver = {
-	.name = "visorhba",
-	.owner = THIS_MODULE,
-	.channel_types = visorhba_channel_types,
-	.probe = visorhba_probe,
-	.remove = visorhba_remove,
-	.pause = visorhba_pause,
-	.resume = visorhba_resume,
-	.channel_interrupt = NULL,
-};
-MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
-MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
-
 struct visor_thread_info {
 	struct task_struct *task;
 	struct completion has_stopped;
@@ -137,8 +121,42 @@ struct visorhba_devdata {
 	int devnum;
 	struct visor_thread_info threadinfo;
 	int thread_wait_ms;
+	struct tasklet_struct tasklet;
+};
+
+/**	visorhba_isr - function to handle interrupts from visorbus
+ *	@dev: device that received interrupt, could be shared
+ *
+ *	Interrupts from visorbus are processed here. Since the IOVM
+ *	sends us "real" interrupts, we are interrupt context here, so
+ *	we need to schedule the work (if any). Note: Interrupts might
+ *	be shared between devices.
+ */
+static void visorhba_isr(struct visor_device *dev)
+{
+	struct visorhba_devdata *devdata = dev_get_drvdata(&dev->device);
+
+	tasklet_schedule(&devdata->tasklet);
+}
+
+/* This is used to tell the visor bus driver which types of visor devices
+ * we support, and what functions to call when a visor device that we support
+ * is attached or removed.
+ */
+static struct visor_driver visorhba_driver = {
+	.name = "visorhba",
+	.owner = THIS_MODULE,
+	.channel_types = visorhba_channel_types,
+	.probe = visorhba_probe,
+	.remove = visorhba_remove,
+	.pause = visorhba_pause,
+	.resume = visorhba_resume,
+	.channel_interrupt = visorhba_isr,
 };
 
+MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
+MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
+
 struct visorhba_devices_open {
 	struct visorhba_devdata *devdata;
 };
@@ -150,31 +168,6 @@ static struct visorhba_devices_open visorhbas_open[VISORHBA_OPEN_MAX];
 		if ((iter->channel == match->channel) &&		  \
 		    (iter->id == match->id) &&			  \
 		    (iter->lun == match->lun))
-/**
- *	visor_thread_start - starts a thread for the device
- *	@thrinfo: The thread to start
- *	@threadfn: Function the thread starts
- *	@thrcontext: Context to pass to the thread, i.e. devdata
- *	@name: string describing name of thread
- *
- *	Starts a thread for the device.
- *
- *	Return 0 on success;
- */
-static int visor_thread_start(struct visor_thread_info *thrinfo,
-			      int (*threadfn)(void *),
-			      void *thrcontext, char *name)
-{
-	/* used to stop the thread */
-	init_completion(&thrinfo->has_stopped);
-	thrinfo->task = kthread_run(threadfn, thrcontext, name);
-	if (IS_ERR(thrinfo->task)) {
-		thrinfo->id = 0;
-		return PTR_ERR(thrinfo->task);
-	}
-	thrinfo->id = thrinfo->task->pid;
-	return 0;
-}
 
 /**
  *	add_scsipending_entry - save off io command that is pending in
@@ -684,10 +677,11 @@ static void visorhba_serverdown_complete(struct visorhba_devdata *devdata)
 	struct uiscmdrsp *cmdrsp;
 	unsigned long flags;
 
+	visorbus_disable_channel_interrupts(devdata->dev);
 	/* Stop using the IOVM response queue (queue should be drained
 	 * by the end)
 	 */
-	kthread_stop(devdata->threadinfo.task);
+	tasklet_disable(&devdata->tasklet);
 
 	/* Fail commands that weren't completed */
 	spin_lock_irqsave(&devdata->privlock, flags);
@@ -1007,28 +1001,21 @@ drain_queue(struct uiscmdrsp *cmdrsp, struct visorhba_devdata *devdata)
  *	from the IO Service Partition. When the queue is empty, wait
  *	to check to see if it is full again.
  */
-static int process_incoming_rsps(void *v)
+static void process_incoming_rsps(unsigned long v)
 {
-	struct visorhba_devdata *devdata = v;
+	struct visorhba_devdata *devdata = (struct visorhba_devdata *)v;
 	struct uiscmdrsp *cmdrsp = NULL;
 	const int size = sizeof(*cmdrsp);
 
 	cmdrsp = kmalloc(size, GFP_ATOMIC);
 	if (!cmdrsp)
-		return -ENOMEM;
+		return;
+
+	/* drain queue */
+	drain_queue(cmdrsp, devdata);
 
-	while (1) {
-		if (kthread_should_stop())
-			break;
-		wait_event_interruptible_timeout(
-			devdata->rsp_queue, (atomic_read(
-					     &devdata->interrupt_rcvd) == 1),
-				msecs_to_jiffies(devdata->thread_wait_ms));
-		/* drain queue */
-		drain_queue(cmdrsp, devdata);
-	}
 	kfree(cmdrsp);
-	return 0;
+	return;
 }
 
 /**
@@ -1072,8 +1059,8 @@ static int visorhba_resume(struct visor_device *dev,
 	if (devdata->serverdown && !devdata->serverchangingstate)
 		devdata->serverchangingstate = 1;
 
-	visor_thread_start(&devdata->threadinfo, process_incoming_rsps,
-			   devdata, "vhba_incming");
+	tasklet_enable(&devdata->tasklet);
+	visorbus_enable_channel_interrupts(dev);
 
 	devdata->serverdown = false;
 	devdata->serverchangingstate = false;
@@ -1149,8 +1136,10 @@ static int visorhba_probe(struct visor_device *dev)
 		goto err_scsi_remove_host;
 
 	devdata->thread_wait_ms = 2;
-	visor_thread_start(&devdata->threadinfo, process_incoming_rsps,
-			   devdata, "vhba_incoming");
+	tasklet_init(&devdata->tasklet, process_incoming_rsps,
+		     (unsigned long)devdata);
+
+	visorbus_enable_channel_interrupts(dev);
 
 	scsi_scan_host(scsihost);
 
@@ -1180,7 +1169,8 @@ static void visorhba_remove(struct visor_device *dev)
 		return;
 
 	scsihost = devdata->scsihost;
-	kthread_stop(devdata->threadinfo.task);
+	visorbus_disable_channel_interrupts(dev);
+	tasklet_kill(&devdata->tasklet);
 	scsi_remove_host(scsihost);
 	scsi_host_put(scsihost);
 
-- 
2.5.0

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

* [PATCH 04/14] staging: unisys: Convert visornic to use visorbus channel interrupt code
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (2 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 03/14] staging: unisys: visorhba: Convert visorhba to use visorbus channel interrupts Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 05/14] staging: unisys: visorinput: use spinlock for channel_interrupt() locking Benjamin Romer
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Instead of having our own timer to set off a interrupt, use the shared code
from visorbus. When visorbus gets s-Par interrupts working we will
automatically get support from it.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 59 +++++++++----------------
 1 file changed, 21 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 296b11c..9bf5bfd 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -43,6 +43,7 @@ static int visornic_pause(struct visor_device *dev,
 			  visorbus_state_complete_func complete_func);
 static int visornic_resume(struct visor_device *dev,
 			   visorbus_state_complete_func complete_func);
+static void visornic_irq(struct visor_device *v);
 
 /* DEBUGFS declarations */
 static ssize_t info_debugfs_read(struct file *file, char __user *buf,
@@ -92,7 +93,7 @@ static struct visor_driver visornic_driver = {
 	.remove = visornic_remove,
 	.pause = visornic_pause,
 	.resume = visornic_resume,
-	.channel_interrupt = NULL,
+	.channel_interrupt = visornic_irq,
 };
 
 struct chanstat {
@@ -117,7 +118,6 @@ struct visornic_devdata {
 	struct visor_device *dev;
 	struct net_device *netdev;
 	struct net_device_stats net_stats;
-	atomic_t interrupt_rcvd;
 	wait_queue_head_t rsp_queue;
 	struct sk_buff **rcvbuf;
 	u64 incarnation_id;		/* lets IOPART know about re-birth */
@@ -190,13 +190,11 @@ struct visornic_devdata {
 
 	int queuefullmsg_logged;
 	struct chanstat chstat;
-	struct timer_list irq_poll_timer;
 	struct napi_struct napi;
 	struct uiscmdrsp cmdrsp[SIZEOF_CMDRSP];
 };
 
 static int visornic_poll(struct napi_struct *napi, int budget);
-static void poll_for_irq(unsigned long v);
 
 /**
  *	visor_copy_fragsinfo_from_skb(
@@ -326,7 +324,7 @@ visornic_serverdown_complete(struct visornic_devdata *devdata)
 	netdev = devdata->netdev;
 
 	/* Stop polling for interrupts */
-	del_timer_sync(&devdata->irq_poll_timer);
+	visorbus_disable_channel_interrupts(devdata->dev);
 
 	rtnl_lock();
 	dev_close(netdev);
@@ -1618,8 +1616,6 @@ service_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata,
 	unsigned long flags;
 	struct net_device *netdev;
 
-	/* TODO: CLIENT ACQUIRE -- Don't really need this at the
-	 * moment */
 	for (;;) {
 		if (!visorchannel_signalremove(devdata->dev->visorchannel,
 					       IOCHAN_FROM_IOPART,
@@ -1722,7 +1718,7 @@ static int visornic_poll(struct napi_struct *napi, int budget)
 }
 
 /**
- *	poll_for_irq	- Checks the status of the response queue.
+ *	visornic_irq	- Checks the status of the response queue.
  *	@v: void pointer to the visronic devdata
  *
  *	Main function of the vnic_incoming thread. Peridocially check the
@@ -1730,19 +1726,13 @@ static int visornic_poll(struct napi_struct *napi, int budget)
  *	Returns when thread has stopped.
  */
 static void
-poll_for_irq(unsigned long v)
+visornic_irq(struct visor_device *v)
 {
-	struct visornic_devdata *devdata = (struct visornic_devdata *)v;
+	struct visornic_devdata *devdata = dev_get_drvdata(&v->device);
 
-	if (!visorchannel_signalempty(
-				   devdata->dev->visorchannel,
-				   IOCHAN_FROM_IOPART))
+	if (!visorchannel_signalempty(devdata->dev->visorchannel,
+				      IOCHAN_FROM_IOPART))
 		napi_schedule(&devdata->napi);
-
-	atomic_set(&devdata->interrupt_rcvd, 0);
-
-	mod_timer(&devdata->irq_poll_timer, msecs_to_jiffies(2));
-
 }
 
 /**
@@ -1861,19 +1851,6 @@ static int visornic_probe(struct visor_device *dev)
 		goto cleanup_xmit_cmdrsp;
 	}
 
-	/* TODO: Setup Interrupt information */
-	/* Let's start our threads to get responses */
-	netif_napi_add(netdev, &devdata->napi, visornic_poll, 64);
-
-	setup_timer(&devdata->irq_poll_timer, poll_for_irq,
-		    (unsigned long)devdata);
-	/*
-	 * Note: This time has to start running before the while
-	 * loop below because the napi routine is responsible for
-	 * setting enab_dis_acked
-	 */
-	mod_timer(&devdata->irq_poll_timer, msecs_to_jiffies(2));
-
 	channel_offset = offsetof(struct spar_io_channel_protocol,
 				  channel_header.features);
 	err = visorbus_read_channel(dev, channel_offset, &features, 8);
@@ -1894,6 +1871,16 @@ static int visornic_probe(struct visor_device *dev)
 		goto cleanup_napi_add;
 	}
 
+	/* Let's start our threads to get responses */
+	netif_napi_add(netdev, &devdata->napi, visornic_poll, 64);
+
+	/*
+	 * Note: Interupts have to be enable before the while
+	 * loop below because the napi routine is responsible for
+	 * setting enab_dis_acked
+	 */
+	visorbus_enable_channel_interrupts(dev);
+
 	err = register_netdev(netdev);
 	if (err) {
 		dev_err(&dev->device,
@@ -1920,7 +1907,7 @@ cleanup_register_netdev:
 	unregister_netdev(netdev);
 
 cleanup_napi_add:
-	del_timer_sync(&devdata->irq_poll_timer);
+	visorbus_disable_channel_interrupts(dev);
 	netif_napi_del(&devdata->napi);
 
 cleanup_xmit_cmdrsp:
@@ -1991,7 +1978,7 @@ static void visornic_remove(struct visor_device *dev)
 
 	unregister_netdev(netdev);  /* this will call visornic_close() */
 
-	del_timer_sync(&devdata->irq_poll_timer);
+	visorbus_disable_channel_interrupts(dev);
 	netif_napi_del(&devdata->napi);
 
 	dev_set_drvdata(&dev->device, NULL);
@@ -2063,11 +2050,7 @@ static int visornic_resume(struct visor_device *dev,
 	devdata->server_change_state = true;
 	spin_unlock_irqrestore(&devdata->priv_lock, flags);
 
-	/* Must transition channel to ATTACHED state BEFORE
-	 * we can start using the device again.
-	 * TODO: State transitions
-	 */
-	mod_timer(&devdata->irq_poll_timer, msecs_to_jiffies(2));
+	visorbus_enable_channel_interrupts(dev);
 
 	init_rcv_bufs(netdev, devdata);
 
-- 
2.5.0

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

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

* [PATCH 05/14] staging: unisys: visorinput: use spinlock for channel_interrupt() locking
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (3 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 04/14] staging: unisys: Convert visornic to use visorbus channel interrupt code Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 06/14] staging: unisys: Only process up to budget amount of responses Benjamin Romer
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh
  Cc: driverdev-devel, sparmaintainer, Tim Sell, David Kershner,
	Benjamin Romer

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

Because visorinput_channel_interrupt() is now called from interrupt
context, we can't use our lock_visor_dev semaphore there.  Instead, we
use a new lock_isr spinlock, which is essentially needed to prevent
visorinput_dev from disappearing while we're accessing it in
visorinput_channel_interrupt().

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorinput/visorinput.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index 57193b3..1c2a210 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -102,6 +102,7 @@ struct visorinput_devdata {
 	struct visor_device *dev;
 	enum visorinput_device_type devtype;
 	struct rw_semaphore lock_visor_dev; /* lock for dev */
+	spinlock_t lock_isr; /* for data accessed in isr */
 	struct input_dev *visorinput_dev;
 	bool paused;
 	struct workqueue_struct *wq;
@@ -417,6 +418,7 @@ static void devdata_put(struct visorinput_devdata *devdata)
 
 static void async_change_resolution(struct work_struct *work)
 {
+	struct input_dev *visorinput_dev = NULL;
 	struct change_resolution_work *p_change_resolution_work =
 		container_of(work, struct change_resolution_work, work);
 	struct visorinput_devdata *devdata =
@@ -425,13 +427,20 @@ static void async_change_resolution(struct work_struct *work)
 			     change_resolution_work_data);
 
 	down_write(&devdata->lock_visor_dev);
+	spin_lock(&devdata->lock_isr);
 
-	if (devdata->paused) /* don't touch device/channel when paused */
-		goto out_locked;
-	if (!devdata->visorinput_dev)
-		goto out_locked;
+	/* devdata->visorinput_dev can only go NULL when lock_isr is held */
 
-	unregister_client_input(devdata->visorinput_dev);
+	if (devdata->paused || (!devdata->visorinput_dev)) {
+		spin_unlock(&devdata->lock_isr);
+		goto out;
+	}
+	visorinput_dev = devdata->visorinput_dev; /* can't unreg with lock */
+	devdata->visorinput_dev = NULL;
+
+	spin_unlock(&devdata->lock_isr);
+
+	unregister_client_input(visorinput_dev);
 	/*
 	 * input_set_abs_params is only effective prior to
 	 * input_register_device().
@@ -448,7 +457,7 @@ static void async_change_resolution(struct work_struct *work)
 	dev_info(&devdata->dev->device, "created mouse %s\n",
 		 dev_name(&devdata->visorinput_dev->dev));
 
-out_locked:
+out:
 	up_write(&devdata->lock_visor_dev);
 	devdata_put(devdata);  /* from schedule_mouse_resolution_change() */
 }
@@ -481,6 +490,7 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
 	INIT_WORK(&devdata->change_resolution_work_data.work,
 		  async_change_resolution);
 	init_rwsem(&devdata->lock_visor_dev);
+	spin_lock_init(&devdata->lock_isr);
 	down_write(&devdata->lock_visor_dev);
 
 	/*
@@ -665,7 +675,7 @@ visorinput_channel_interrupt(struct visor_device *dev)
 	if (!devdata)
 		return;
 
-	down_write(&devdata->lock_visor_dev);
+	spin_lock(&devdata->lock_isr);
 	if (devdata->paused) /* don't touch device/channel when paused */
 		goto out_locked;
 
@@ -771,7 +781,7 @@ visorinput_channel_interrupt(struct visor_device *dev)
 	}
 out_locked:
 	devdata_put(devdata);
-	up_write(&devdata->lock_visor_dev);
+	spin_unlock(&devdata->lock_isr);
 }
 
 static int
-- 
2.5.0

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

* [PATCH 06/14] staging: unisys: Only process up to budget amount of responses
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (4 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 05/14] staging: unisys: visorinput: use spinlock for channel_interrupt() locking Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 07/14] staging: unisys: Add support to update Features bits in channel queues Benjamin Romer
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, David Kershner, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

>From napi documentation you should only process the amount your
budget allows, if you go over it just wait for the next napi poll
to continue.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 9bf5bfd..9566b91 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -36,6 +36,7 @@
  *         = 163840 bytes
  */
 #define MAX_BUF 163840
+#define NAPI_WEIGHT 64
 
 static int visornic_probe(struct visor_device *dev);
 static void visornic_remove(struct visor_device *dev);
@@ -1611,12 +1612,12 @@ drain_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata)
  */
 static void
 service_resp_queue(struct uiscmdrsp *cmdrsp, struct visornic_devdata *devdata,
-		   int *rx_work_done)
+		   int *rx_work_done, int budget)
 {
 	unsigned long flags;
 	struct net_device *netdev;
 
-	for (;;) {
+	while (*rx_work_done < budget) {
 		if (!visorchannel_signalremove(devdata->dev->visorchannel,
 					       IOCHAN_FROM_IOPART,
 					       cmdrsp))
@@ -1705,7 +1706,7 @@ static int visornic_poll(struct napi_struct *napi, int budget)
 	int rx_count = 0;
 
 	send_rcv_posts_if_needed(devdata);
-	service_resp_queue(devdata->cmdrsp, devdata, &rx_count);
+	service_resp_queue(devdata->cmdrsp, devdata, &rx_count, budget);
 
 	/*
 	 * If there aren't any more packets to receive
@@ -1872,7 +1873,7 @@ static int visornic_probe(struct visor_device *dev)
 	}
 
 	/* Let's start our threads to get responses */
-	netif_napi_add(netdev, &devdata->napi, visornic_poll, 64);
+	netif_napi_add(netdev, &devdata->napi, visornic_poll, NAPI_WEIGHT);
 
 	/*
 	 * Note: Interupts have to be enable before the while
-- 
2.5.0

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

* [PATCH 07/14] staging: unisys: Add support to update Features bits in channel queues
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (5 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 06/14] staging: unisys: Only process up to budget amount of responses Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 08/14] staging: unisys: Add channel feature access functions Benjamin Romer
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, David Kershner, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Add support to visorbus to update the features in the channel queues.
Signal queues features is the memory location to disable/enable signal
queue interrupts.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h      |  4 +++
 drivers/staging/unisys/visorbus/visorchannel.c | 46 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 9235536..644ca30 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -218,6 +218,10 @@ 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);
+void visorchannel_set_sig_features(struct visorchannel *channel, u32 queue,
+				   u64 features);
+void visorchannel_clear_sig_features(struct visorchannel *channel, u32 queue,
+				     u64 features);
 
 #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/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index a4e117f..2ef79fa 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -538,6 +538,52 @@ visorchannel_signalqueue_max_slots(struct visorchannel *channel, u32 queue)
 }
 EXPORT_SYMBOL_GPL(visorchannel_signalqueue_max_slots);
 
+/**
+ *	visorchannel_clear_or_set_sig_features
+ *
+ *	Clear or set bits within the 64-bit features word for the
+ *	specified channel and queue.
+ *	@channel: the channel to modify features for.
+ *	@queue: the queue number to modify features for.
+ *	@features: a mask of feature bits usage based on is_set flag;
+ *	@is_set: if is_set is true, 1 bits indicate which features bits
+ *		 you want to set within the feature word;
+ *		 if is_set is false, 1 bits indicate which feature bits
+ *		 you want to clear within the features word
+ */
+void
+visorchannel_clear_or_set_sig_features(struct visorchannel *channel,
+				       u32 queue, u64 features, bool is_set)
+{
+	struct signal_queue_header sig_hdr;
+
+	if (!sig_read_header(channel, queue, &sig_hdr))
+		return;
+	sig_hdr.features = (is_set) ? sig_hdr.features | features :
+			   sig_hdr.features & !features;
+	if (!SIG_WRITE_FIELD(channel, queue, &sig_hdr, features))
+		return;
+	wmb(); /* required to sync channel with IOSP */
+}
+
+void
+visorchannel_clear_sig_features(struct visorchannel *channel, u32 queue,
+				u64 features)
+{
+	visorchannel_clear_or_set_sig_features(channel, queue, features,
+					       false);
+}
+EXPORT_SYMBOL_GPL(visorchannel_clear_sig_features);
+
+void
+visorchannel_set_sig_features(struct visorchannel *channel, u32 queue,
+			      u64 features)
+{
+	visorchannel_clear_or_set_sig_features(channel, queue, features,
+					       true);
+}
+EXPORT_SYMBOL_GPL(visorchannel_set_sig_features);
+
 static void
 sigqueue_debug(struct signal_queue_header *q, int which, struct seq_file *seq)
 {
-- 
2.5.0

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

* [PATCH 08/14] staging: unisys: Add channel feature access functions
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (6 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 07/14] staging: unisys: Add support to update Features bits in channel queues Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 09/14] staging: unisys: Re-enable interrupts after we have done the work Benjamin Romer
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Need access functions to set channel polling

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 55 +++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 63e7863..4304ca0 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -977,6 +977,61 @@ visorbus_disable_channel_interrupts(struct visor_device *dev)
 }
 EXPORT_SYMBOL_GPL(visorbus_disable_channel_interrupts);
 
+int visorbus_set_channel_features(struct visor_device *dev, u64 feature_bits)
+{
+	int channel_offset = 0, err = 0;
+	u64 features;
+
+	channel_offset = offsetof(struct channel_header,
+				  features);
+	err = visorbus_read_channel(dev, channel_offset, &features, 8);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to get features from chan (%d)\n",
+			__func__, err);
+		return err;
+	}
+
+	features |= (feature_bits);
+
+	err = visorbus_write_channel(dev, channel_offset, &features, 8);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to get features from chan (%d)\n",
+			__func__, err);
+		return err;
+	}
+	return err;
+}
+
+int visorbus_clear_channel_features(struct visor_device *dev, u64 feature_bits)
+{
+	int channel_offset = 0, err = 0;
+	u64 features, mask;
+
+	channel_offset = offsetof(struct channel_header,
+				  features);
+	err = visorbus_read_channel(dev, channel_offset, &features, 8);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to get features from chan (%d)\n",
+			__func__, err);
+		return err;
+	}
+
+	mask = ~(feature_bits);
+	features &= mask;
+
+	err = visorbus_write_channel(dev, channel_offset, &features, 8);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to get features from chan (%d)\n",
+			__func__, err);
+		return err;
+	}
+	return err;
+}
+
 /** 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
-- 
2.5.0

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

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

* [PATCH 09/14] staging: unisys: Re-enable interrupts after we have done the work
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (7 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 08/14] staging: unisys: Add channel feature access functions Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 10/14] staging: unisys: Capture data from device create to register interrupt Benjamin Romer
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Instead of rescheduling the work queue after we have called the
channel interrupt, start it when the driver has finished processing
the queue.

This patch introduces the visorbus_rearm_channel_interrupts function
that must get called when the driver decides that it is done processing
its queue.

Visorinput, visorhba, and visornic were all updated to call the new
function.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       | 2 ++
 drivers/staging/unisys/visorbus/visorbus_main.c | 9 +++++++--
 drivers/staging/unisys/visorhba/visorhba_main.c | 6 +++++-
 drivers/staging/unisys/visorinput/visorinput.c  | 5 ++++-
 drivers/staging/unisys/visornic/visornic_main.c | 6 +++++-
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 644ca30..5f44289 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -178,6 +178,8 @@ int visorbus_registerdevnode(struct visor_device *dev,
 			     const char *name, int major, int minor);
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
+void visorbus_rearm_channel_interrupts(struct visor_device *dev);
+
 #endif
 
 /* Note that for visorchannel_create()
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 4304ca0..afbb9bc 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -715,6 +715,13 @@ unregister_driver_attributes(struct visor_driver *drv)
 	driver_remove_file(&drv->driver, &drv->version_attr);
 }
 
+visorbus_rearm_channel_interrupts(struct visor_device *dev)
+{
+	if (!visor_periodic_work_nextperiod(dev->periodic_work))
+		put_device(&dev->device);
+}
+EXPORT_SYMBOL_GPL(visorbus_rearm_channel_interrupts);
+
 static void
 dev_periodic_work(void *xdev)
 {
@@ -725,8 +732,6 @@ dev_periodic_work(void *xdev)
 	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);
 }
 
 static void
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 70bc878..0f6bb26 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -1008,12 +1008,16 @@ static void process_incoming_rsps(unsigned long v)
 	const int size = sizeof(*cmdrsp);
 
 	cmdrsp = kmalloc(size, GFP_ATOMIC);
-	if (!cmdrsp)
+	if (!cmdrsp) {
+		visorbus_rearm_channel_interrupts(devdata->dev);
 		return;
+	}
 
 	/* drain queue */
 	drain_queue(cmdrsp, devdata);
 
+	visorbus_rearm_channel_interrupts(devdata->dev);
+
 	kfree(cmdrsp);
 	return;
 }
diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index 1c2a210..84d0511 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -673,7 +673,7 @@ visorinput_channel_interrupt(struct visor_device *dev)
 		devdata_get(dev_get_drvdata(&dev->device));
 
 	if (!devdata)
-		return;
+		goto rearm_interrupts;
 
 	spin_lock(&devdata->lock_isr);
 	if (devdata->paused) /* don't touch device/channel when paused */
@@ -782,6 +782,9 @@ visorinput_channel_interrupt(struct visor_device *dev)
 out_locked:
 	devdata_put(devdata);
 	spin_unlock(&devdata->lock_isr);
+
+rearm_interrupts:
+	visorbus_rearm_channel_interrupts(dev);
 }
 
 static int
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 9566b91..e8ad219 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -1712,8 +1712,10 @@ static int visornic_poll(struct napi_struct *napi, int budget)
 	 * If there aren't any more packets to receive
 	 * stop the poll
 	 */
-	if (rx_count < budget)
+	if (rx_count < budget) {
 		napi_complete(napi);
+		visorbus_rearm_channel_interrupts(devdata->dev);
+	}
 
 	return rx_count;
 }
@@ -1734,6 +1736,8 @@ visornic_irq(struct visor_device *v)
 	if (!visorchannel_signalempty(devdata->dev->visorchannel,
 				      IOCHAN_FROM_IOPART))
 		napi_schedule(&devdata->napi);
+	else
+		visorbus_rearm_channel_interrupts(v);
 }
 
 /**
-- 
2.5.0

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

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

* [PATCH 10/14] staging: unisys: Capture data from device create to register interrupt.
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (8 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 09/14] staging: unisys: Re-enable interrupts after we have done the work Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 11/14] staging: unisys: Don't go into POLLING mode in visornic_probe Benjamin Romer
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Tim Sell, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

s-Par Firmware sends a CREATE_DEVICE information for each device that
gets created. Contained in the CREATE_DEVICE is the GSI interrupt vector
we should request an interrupt on. Save off that information and when
the client driver asks to register for interrupt, register the gsi
number and then request the irq based off that information.

When an interrupt happens, update the flag to inform IO Service Partition
and call the channel_interrupt function. Update the flag in the rearm
function to accept another interrupt.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Tim Sell <timothy.sell@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       |   5 +
 drivers/staging/unisys/visorbus/visorbus_main.c | 118 ++++++++++++++++++++++--
 drivers/staging/unisys/visorbus/visorchannel.c  |   1 +
 drivers/staging/unisys/visorbus/visorchipset.c  |  17 +++-
 drivers/staging/unisys/visorhba/visorhba_main.c |   4 +
 drivers/staging/unisys/visornic/visornic_main.c |   5 +-
 6 files changed, 137 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index 5f44289..e2251f7 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -159,6 +159,9 @@ struct visor_device {
 	u32 switch_no;
 	u32 internal_port_no;
 	uuid_le partition_uuid;
+	int irq;
+	int wait_ms;
+	int recv_queue;		/* specifies which queue to receive msgs on */
 };
 
 #define to_visor_device(x) container_of(x, struct visor_device, device)
@@ -176,6 +179,8 @@ int visorbus_clear_channel(struct visor_device *dev,
 			   unsigned long offset, u8 ch, unsigned long nbytes);
 int visorbus_registerdevnode(struct visor_device *dev,
 			     const char *name, int major, int minor);
+int visorbus_register_for_channel_interrupts(struct visor_device *dev,
+					     u32 queue);
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
 void visorbus_rearm_channel_interrupts(struct visor_device *dev);
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index afbb9bc..a45c6a2 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -14,6 +14,8 @@
  * details.
  */
 
+#include <linux/acpi.h>
+#include <linux/interrupt.h>
 #include <linux/uuid.h>
 
 #include "visorbus.h"
@@ -715,13 +717,6 @@ unregister_driver_attributes(struct visor_driver *drv)
 	driver_remove_file(&drv->driver, &drv->version_attr);
 }
 
-visorbus_rearm_channel_interrupts(struct visor_device *dev)
-{
-	if (!visor_periodic_work_nextperiod(dev->periodic_work))
-		put_device(&dev->device);
-}
-EXPORT_SYMBOL_GPL(visorbus_rearm_channel_interrupts);
-
 static void
 dev_periodic_work(void *xdev)
 {
@@ -969,19 +964,63 @@ EXPORT_SYMBOL_GPL(visorbus_registerdevnode);
  *  interrupt function periodically...
  */
 void
+visorbus_rearm_channel_interrupts(struct visor_device *dev)
+{
+	if (dev->irq)
+		visorchannel_set_sig_features(dev->visorchannel,
+					      dev->recv_queue,
+					      ULTRA_CHANNEL_ENABLE_INTS);
+	else if (!visor_periodic_work_nextperiod(dev->periodic_work))
+		put_device(&dev->device);
+}
+EXPORT_SYMBOL_GPL(visorbus_rearm_channel_interrupts);
+
+void
 visorbus_enable_channel_interrupts(struct visor_device *dev)
 {
-	dev_start_periodic_work(dev);
+	if (dev->irq)
+		visorchannel_set_sig_features(dev->visorchannel,
+					      dev->recv_queue,
+					      ULTRA_CHANNEL_ENABLE_INTS);
+	else
+		dev_start_periodic_work(dev);
 }
 EXPORT_SYMBOL_GPL(visorbus_enable_channel_interrupts);
 
 void
 visorbus_disable_channel_interrupts(struct visor_device *dev)
 {
-	dev_stop_periodic_work(dev);
+	if (!dev->irq)
+		visorchannel_clear_sig_features(dev->visorchannel,
+						dev->recv_queue,
+						ULTRA_CHANNEL_ENABLE_INTS);
+	else
+		dev_stop_periodic_work(dev);
 }
 EXPORT_SYMBOL_GPL(visorbus_disable_channel_interrupts);
 
+static irqreturn_t
+visorbus_isr(int irq, void *dev_id)
+{
+	struct visor_device *dev = (struct visor_device *)dev_id;
+	struct visor_driver *drv = to_visor_driver(dev->device.driver);
+
+	/* Disable the interrupt in hardware for this device.
+	 * When the device is done handling the interrupt, it has
+	 * the responsibility of re-arming the interrupt so the SP
+	 * can send another one.
+	 */
+	visorchannel_clear_sig_features(dev->visorchannel,
+					dev->recv_queue,
+					ULTRA_CHANNEL_ENABLE_INTS);
+	if (drv->channel_interrupt) {
+		drv->channel_interrupt(dev);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 int visorbus_set_channel_features(struct visor_device *dev, u64 feature_bits)
 {
 	int channel_offset = 0, err = 0;
@@ -1037,6 +1076,53 @@ int visorbus_clear_channel_features(struct visor_device *dev, u64 feature_bits)
 	return err;
 }
 
+#define INTERRUPT_VECTOR_MASK 0x3f
+int visorbus_register_for_channel_interrupts(struct visor_device *dev,
+					     u32 queue)
+{
+	int err = 0;
+
+	if (!dev->irq)
+		goto stay_in_polling;
+
+	err = request_irq(dev->irq, visorbus_isr, IRQF_SHARED,
+			  dev_name(&dev->device), dev);
+	if (err < 0) {
+		dev_err(&dev->device,
+			"failed to request irq %d continuing to poll. err = %d",
+			dev->irq, err);
+		goto stay_in_polling;
+	}
+
+	dev_info(&dev->device, "IRQ=%d registered\n", dev->irq);
+
+	err = visorbus_set_channel_features(dev, ULTRA_IO_DRIVER_ENABLES_INTS |
+					    ULTRA_IO_DRIVER_DISABLES_INTS);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to set ENALBES ints from chan (%d)\n",
+			__func__, err);
+		goto stay_in_polling;
+	}
+
+	err = visorbus_clear_channel_features(dev, ULTRA_IO_CHANNEL_IS_POLLING);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to clear POOLING flag from chan (%d)\n",
+			__func__, err);
+		goto stay_in_polling;
+	}
+
+	dev->wait_ms = 2000;
+	dev->recv_queue = queue;
+	return 0;
+
+stay_in_polling:
+	dev->irq = 0;
+	return err;
+}
+EXPORT_SYMBOL_GPL(visorbus_register_for_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
@@ -1086,6 +1172,20 @@ create_visor_device(struct visor_device *dev)
 	dev_set_name(&dev->device, "vbus%u:dev%u",
 		     chipset_bus_no, chipset_dev_no);
 
+	/* Automatically set driver into POLLING mode, if the driver
+	 * wants to use interrupts, it's probe can call
+	 * visorbus_register_for_interrupts.
+	 */
+	rc = visorbus_set_channel_features(dev, ULTRA_IO_CHANNEL_IS_POLLING |
+					   ULTRA_IO_DRIVER_DISABLES_INTS);
+	if (rc) {
+		dev_err(&dev->device,
+			"%s failed to set channel features for chan (%d)\n",
+			__func__, rc);
+		goto away;
+	}
+	dev->wait_ms = 2;
+
 	/*  device_add does this:
 	 *    bus_add_device(dev)
 	 *    ->device_attach(dev)
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 2ef79fa..618784e 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -21,6 +21,7 @@
 
 #include <linux/uuid.h>
 #include <linux/io.h>
+#include <linux/llist.h>
 
 #include "version.h"
 #include "visorbus.h"
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 07594f4..4239606 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1205,6 +1205,7 @@ my_device_create(struct controlvm_message *inmsg)
 	struct visor_device *bus_info;
 	struct visorchannel *visorchannel;
 	int rc = CONTROLVM_RESP_SUCCESS;
+	int gsi_vector;
 
 	bus_info = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE, NULL);
 	if (!bus_info) {
@@ -1237,12 +1238,24 @@ my_device_create(struct controlvm_message *inmsg)
 		goto cleanup;
 	}
 
+	dev_info->device.parent = &bus_info->device;
+
 	dev_info->chipset_bus_no = bus_no;
 	dev_info->chipset_dev_no = dev_no;
 	dev_info->inst = cmd->create_device.dev_inst_uuid;
 
-	/* not sure where the best place to set the 'parent' */
-	dev_info->device.parent = &bus_info->device;
+	gsi_vector = cmd->create_device.intr.recv_irq_handle;
+	if (gsi_vector > 0) {
+		dev_info->irq = acpi_register_gsi(&dev_info->device, gsi_vector,
+						  ACPI_LEVEL_SENSITIVE,
+						  ACPI_ACTIVE_LOW);
+		if (dev_info->irq < 0) {
+			dev_err(&dev_info->device,
+				"Error registering gsi number: %d (%d)",
+				gsi_vector, dev_info->irq);
+			dev_info->irq = 0;
+		}
+	}
 
 	POSTCODE_LINUX_4(DEVICE_CREATE_ENTRY_PC, dev_no, bus_no,
 			 POSTCODE_SEVERITY_INFO);
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 0f6bb26..e87bc2b 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -1143,6 +1143,10 @@ static int visorhba_probe(struct visor_device *dev)
 	tasklet_init(&devdata->tasklet, process_incoming_rsps,
 		     (unsigned long)devdata);
 
+	/* I want to use real interrupts if available so need to
+	 * register
+	 */
+	visorbus_register_for_channel_interrupts(dev, IOCHAN_FROM_IOPART);
 	visorbus_enable_channel_interrupts(dev);
 
 	scsi_scan_host(scsihost);
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index e8ad219..39a97f3 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -1880,10 +1880,11 @@ static int visornic_probe(struct visor_device *dev)
 	netif_napi_add(netdev, &devdata->napi, visornic_poll, NAPI_WEIGHT);
 
 	/*
-	 * Note: Interupts have to be enable before the while
-	 * loop below because the napi routine is responsible for
+	 * Note: Interupts have to be enable before we register
+	 * because the napi routine is responsible for
 	 * setting enab_dis_acked
 	 */
+	visorbus_register_for_channel_interrupts(dev, IOCHAN_FROM_IOPART);
 	visorbus_enable_channel_interrupts(dev);
 
 	err = register_netdev(netdev);
-- 
2.5.0

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

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

* [PATCH 11/14] staging: unisys: Don't go into POLLING mode in visornic_probe.
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (9 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 10/14] staging: unisys: Capture data from device create to register interrupt Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 12/14] staging: unisys: Don't set polling mode in visorhba_probe Benjamin Romer
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, David Kershner, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Since visorbus now supports s-Par interrupts it will
handle the polling/interrupt mode for us.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 39a97f3..25c89b4 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -1866,7 +1866,6 @@ static int visornic_probe(struct visor_device *dev)
 		goto cleanup_napi_add;
 	}
 
-	features |= ULTRA_IO_CHANNEL_IS_POLLING;
 	features |= ULTRA_IO_DRIVER_SUPPORTS_ENHANCED_RCVBUF_CHECKING;
 	err = visorbus_write_channel(dev, channel_offset, &features, 8);
 	if (err) {
-- 
2.5.0

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

* [PATCH 12/14] staging: unisys: Don't set polling mode in visorhba_probe
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (10 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 11/14] staging: unisys: Don't go into POLLING mode in visornic_probe Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 13/14] staging: unisys: Remove semaphores around channel interrupts Benjamin Romer
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

Visorbus handles interrupt states for the drivers now, don't need
to handle it in the driver.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorhba/visorhba_main.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index e87bc2b..319ccfd 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -1085,7 +1085,6 @@ static int visorhba_probe(struct visor_device *dev)
 	struct vhba_config_max max;
 	struct visorhba_devdata *devdata = NULL;
 	int i, err, channel_offset;
-	u64 features;
 
 	scsihost = scsi_host_alloc(&visorhba_driver_template,
 				   sizeof(*devdata));
@@ -1129,16 +1128,6 @@ static int visorhba_probe(struct visor_device *dev)
 	devdata->serverchangingstate = false;
 	devdata->scsihost = scsihost;
 
-	channel_offset = offsetof(struct spar_io_channel_protocol,
-				  channel_header.features);
-	err = visorbus_read_channel(dev, channel_offset, &features, 8);
-	if (err)
-		goto err_scsi_remove_host;
-	features |= ULTRA_IO_CHANNEL_IS_POLLING;
-	err = visorbus_write_channel(dev, channel_offset, &features, 8);
-	if (err)
-		goto err_scsi_remove_host;
-
 	devdata->thread_wait_ms = 2;
 	tasklet_init(&devdata->tasklet, process_incoming_rsps,
 		     (unsigned long)devdata);
@@ -1153,9 +1142,6 @@ static int visorhba_probe(struct visor_device *dev)
 
 	return 0;
 
-err_scsi_remove_host:
-	scsi_remove_host(scsihost);
-
 err_scsi_host_put:
 	scsi_host_put(scsihost);
 	return err;
-- 
2.5.0

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

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

* [PATCH 13/14] staging: unisys: Remove semaphores around channel interrupts.
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (11 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 12/14] staging: unisys: Don't set polling mode in visorhba_probe Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 14:58 ` [PATCH 14/14] staging: unisys: Allow for unregistering of interrupts Benjamin Romer
  2015-11-17 22:18 ` [PATCH 00/14] staging: unisys: add channel interrupt support Greg KH
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh; +Cc: sparmaintainer, driverdev-devel, Tim Sell, Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

We can remove the semaphore from around the interrupt callback in
dev_periodic_work().

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Tim Sell <timothy.sell@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index a45c6a2..962af12 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -723,10 +723,8 @@ dev_periodic_work(void *xdev)
 	struct visor_device *dev = xdev;
 	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);
 }
 
 static void
-- 
2.5.0

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

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

* [PATCH 14/14] staging: unisys: Allow for unregistering of interrupts.
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (12 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 13/14] staging: unisys: Remove semaphores around channel interrupts Benjamin Romer
@ 2015-11-17 14:58 ` Benjamin Romer
  2015-11-17 22:18 ` [PATCH 00/14] staging: unisys: add channel interrupt support Greg KH
  14 siblings, 0 replies; 19+ messages in thread
From: Benjamin Romer @ 2015-11-17 14:58 UTC (permalink / raw)
  To: gregkh
  Cc: driverdev-devel, sparmaintainer, David Kershner, Tim Sell,
	Benjamin Romer

From: David Kershner <david.kershner@unisys.com>

The new interrupt code was NOT distinguishing between the availability of
an irq (i.e., visor_device.irq != 0), and the fact that we were in fact
operating in real interrupt mode (i.e., request_irq() succeeded).  This
could cause us to do the wrong thing in visorbus_enable_channel_interrupts,
visorbus_disable_channel_interrupts and visorbus_rearm_channel_interrupts.
This was fixed by adding a boolean named visor_device.irq_requested, which
is true iff request_irq() succeeded.

We were not properly restoring interrupt-related state after a failed
attempt of visorbus_register_channel_interrupts(), nor after the device
was unattached from a function driver.  Most-notably, we did NOT call
free_irq() (the book-end to request_irq()).  This was fixed via a new
visorbus_unregister_for_channel_interrupts.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Tim Sell <timothy.sell@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
 drivers/staging/unisys/include/visorbus.h       |  2 +
 drivers/staging/unisys/visorbus/visorbus_main.c | 65 ++++++++++++++++++++++---
 2 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index e2251f7..23f4704 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -162,6 +162,7 @@ struct visor_device {
 	int irq;
 	int wait_ms;
 	int recv_queue;		/* specifies which queue to receive msgs on */
+	bool irq_requested;     /* true iff request_irq() succeeded */
 };
 
 #define to_visor_device(x) container_of(x, struct visor_device, device)
@@ -181,6 +182,7 @@ int visorbus_registerdevnode(struct visor_device *dev,
 			     const char *name, int major, int minor);
 int visorbus_register_for_channel_interrupts(struct visor_device *dev,
 					     u32 queue);
+int visorbus_unregister_for_channel_interrupts(struct visor_device *dev);
 void visorbus_enable_channel_interrupts(struct visor_device *dev);
 void visorbus_disable_channel_interrupts(struct visor_device *dev);
 void visorbus_rearm_channel_interrupts(struct visor_device *dev);
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 962af12..27aa3df 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -723,6 +723,14 @@ dev_periodic_work(void *xdev)
 	struct visor_device *dev = xdev;
 	struct visor_driver *drv = to_visor_driver(dev->device.driver);
 
+	/*
+	 * Note that channel_interrupt() is called withOUT
+	 * visordriver_callback_lock(), unlike the other callbacks.  This both
+	 * makes the behavior consistent between polling versus NON-polling
+	 * modes, and enables us to handle I/Os while in the function driver's
+	 * probe() function, which is necessary particularly for the
+	 * scsi_scan_host() call in the visorhba case.
+	 */
 	if (drv->channel_interrupt)
 		drv->channel_interrupt(dev);
 }
@@ -734,8 +742,10 @@ dev_start_periodic_work(struct visor_device *dev)
 		return;
 	/* now up by at least 2 */
 	get_device(&dev->device);
-	if (!visor_periodic_work_start(dev->periodic_work))
+	if (!visor_periodic_work_start(dev->periodic_work)) {
+		dev_err(&dev->device, "%s failed\n", __func__);
 		put_device(&dev->device);
+	}
 }
 
 static void
@@ -786,13 +796,18 @@ visordriver_probe_device(struct device *xdev)
 	get_device(&dev->device);
 	visorbus_set_channel_state(dev, CHANNELCLI_OWNED);
 	if (!drv->probe) {
+		dev_err(xdev, "%s function driver provide no probe()\n",
+			__func__);
 		up(&dev->visordriver_callback_lock);
 		rc = -1;
 		goto away;
 	}
 	rc = drv->probe(dev);
-	if (rc < 0)
+	if (rc < 0) {
+		dev_err(xdev, "%s function driver probe() errored\n",
+			__func__);
 		goto away;
+	}
 
 	fix_vbus_dev_info(dev);
 	up(&dev->visordriver_callback_lock);
@@ -830,6 +845,7 @@ visordriver_remove_device(struct device *xdev)
 	}
 	up(&dev->visordriver_callback_lock);
 	dev_stop_periodic_work(dev);
+	visorbus_unregister_for_channel_interrupts(dev);
 	devmajorminor_remove_all_files(dev);
 
 	visorbus_set_channel_state(dev, CHANNELCLI_ATTACHED);
@@ -964,7 +980,7 @@ EXPORT_SYMBOL_GPL(visorbus_registerdevnode);
 void
 visorbus_rearm_channel_interrupts(struct visor_device *dev)
 {
-	if (dev->irq)
+	if (dev->irq_requested)
 		visorchannel_set_sig_features(dev->visorchannel,
 					      dev->recv_queue,
 					      ULTRA_CHANNEL_ENABLE_INTS);
@@ -976,24 +992,30 @@ EXPORT_SYMBOL_GPL(visorbus_rearm_channel_interrupts);
 void
 visorbus_enable_channel_interrupts(struct visor_device *dev)
 {
-	if (dev->irq)
+	if (dev->irq_requested) {
+		dev_dbg(&dev->device, "%s real interrupts\n", __func__);
 		visorchannel_set_sig_features(dev->visorchannel,
 					      dev->recv_queue,
 					      ULTRA_CHANNEL_ENABLE_INTS);
-	else
+	} else {
+		dev_dbg(&dev->device, "%s polling\n", __func__);
 		dev_start_periodic_work(dev);
+	}
 }
 EXPORT_SYMBOL_GPL(visorbus_enable_channel_interrupts);
 
 void
 visorbus_disable_channel_interrupts(struct visor_device *dev)
 {
-	if (!dev->irq)
+	if (!dev->irq_requested) {
+		dev_dbg(&dev->device, "%s real interrupts\n", __func__);
 		visorchannel_clear_sig_features(dev->visorchannel,
 						dev->recv_queue,
 						ULTRA_CHANNEL_ENABLE_INTS);
-	else
+	} else {
+		dev_dbg(&dev->device, "%s polling\n", __func__);
 		dev_stop_periodic_work(dev);
+	}
 }
 EXPORT_SYMBOL_GPL(visorbus_disable_channel_interrupts);
 
@@ -1074,6 +1096,32 @@ int visorbus_clear_channel_features(struct visor_device *dev, u64 feature_bits)
 	return err;
 }
 
+int visorbus_unregister_for_channel_interrupts(struct visor_device *dev)
+{
+	int err = 0;
+
+	if (dev->irq_requested) {
+		free_irq(dev->irq, dev);
+		dev->irq_requested = false;
+	}
+	err = visorbus_set_channel_features(dev, ULTRA_IO_CHANNEL_IS_POLLING);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to set polling flag from chan (%d)\n",
+			__func__, err);
+	}
+	err = visorbus_clear_channel_features(dev,
+					      ULTRA_IO_DRIVER_ENABLES_INTS |
+					      ULTRA_IO_DRIVER_DISABLES_INTS);
+	if (err) {
+		dev_err(&dev->device,
+			"%s failed to clear enable ints from chan (%d)\n",
+			__func__, err);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(visorbus_unregister_for_channel_interrupts);
+
 #define INTERRUPT_VECTOR_MASK 0x3f
 int visorbus_register_for_channel_interrupts(struct visor_device *dev,
 					     u32 queue)
@@ -1093,6 +1141,7 @@ int visorbus_register_for_channel_interrupts(struct visor_device *dev,
 	}
 
 	dev_info(&dev->device, "IRQ=%d registered\n", dev->irq);
+	dev->irq_requested = true;
 
 	err = visorbus_set_channel_features(dev, ULTRA_IO_DRIVER_ENABLES_INTS |
 					    ULTRA_IO_DRIVER_DISABLES_INTS);
@@ -1116,7 +1165,7 @@ int visorbus_register_for_channel_interrupts(struct visor_device *dev,
 	return 0;
 
 stay_in_polling:
-	dev->irq = 0;
+	visorbus_unregister_for_channel_interrupts(dev);
 	return err;
 }
 EXPORT_SYMBOL_GPL(visorbus_register_for_channel_interrupts);
-- 
2.5.0

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

* Re: [PATCH 00/14] staging: unisys: add channel interrupt support
  2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
                   ` (13 preceding siblings ...)
  2015-11-17 14:58 ` [PATCH 14/14] staging: unisys: Allow for unregistering of interrupts Benjamin Romer
@ 2015-11-17 22:18 ` Greg KH
  2015-11-20 18:59   ` Ben Romer
  14 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2015-11-17 22:18 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: sparmaintainer, driverdev-devel

On Tue, Nov 17, 2015 at 09:57:58AM -0500, Benjamin Romer wrote:
> This patch series adds a centralized infrastructure and device support
> for channel interrupts sent to s-Par virtual devices. With these changes,
> the visorhba device is ~80% faster than with only polling, and visornic
> receives a speedup of over 3500% (from ~9Mb/s to between 360Mb/s and
> 390Mb/s).

Why are you adding new functions to the code before it is merged out of
staging?  Please don't do that, please get the code out of staging
first.

thanks,

greg k-h

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

* Re: [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work
  2015-11-17 14:57 ` [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work Benjamin Romer
@ 2015-11-17 22:19   ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2015-11-17 22:19 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: sparmaintainer, driverdev-devel

On Tue, Nov 17, 2015 at 09:57:59AM -0500, Benjamin Romer wrote:
> From: David Kershner <david.kershner@unisys.com>
> 
> Use ms_to_jiffies for the periodic work queue instead of raw jiffies.
> 
> Signed-off-by: David Kershner <david.kershner@unisys.com>
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
> index a272b48..1fa6e5b 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -34,8 +34,8 @@ static int visorbus_debugref;
>  #define SERIALLOOPBACKCHANADDR (100 * 1024 * 1024)
>  
>  #define CURRENT_FILE_PC VISOR_BUS_PC_visorbus_main_c
> -#define POLLJIFFIES_TESTWORK         100
> -#define POLLJIFFIES_NORMALCHANNEL     10
> +#define POLLJIFFIES_TESTWORK         msecs_to_jiffies(100)
> +#define POLLJIFFIES_NORMALCHANNEL     msecs_to_jiffies(2)
>  
>  static int busreg_rc = -ENODEV; /* stores the result from bus registration */
>  

Isn't this a bugfix?  Should it go into 4.4-final and maybe older stable
kernels?

thanks,

greg k-h

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

* Re: [PATCH 00/14] staging: unisys: add channel interrupt support
  2015-11-17 22:18 ` [PATCH 00/14] staging: unisys: add channel interrupt support Greg KH
@ 2015-11-20 18:59   ` Ben Romer
  2015-11-20 21:51     ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Ben Romer @ 2015-11-20 18:59 UTC (permalink / raw)
  To: Greg KH; +Cc: sparmaintainer, driverdev-devel

On 11/17/2015 05:18 PM, Greg KH wrote:
> On Tue, Nov 17, 2015 at 09:57:58AM -0500, Benjamin Romer wrote:
>> This patch series adds a centralized infrastructure and device support
>> for channel interrupts sent to s-Par virtual devices. With these changes,
>> the visorhba device is ~80% faster than with only polling, and visornic
>> receives a speedup of over 3500% (from ~9Mb/s to between 360Mb/s and
>> 390Mb/s).
>
> Why are you adding new functions to the code before it is merged out of
> staging?  Please don't do that, please get the code out of staging
> first.
>
> thanks,
>
> greg k-h
>

Hi Greg,

Interrupt support was originally part of the driver set when we 
submitted it, but we removed it when we renamed the drivers and 
restructured the set. The work was listed in the TODO so I thought it 
would be okay to patch it in staging, but if we should wait until the 
drivers are out, then we will wait. :)

Do you think our drivers are clean enough to be moved out soon? None of 
the files generate checkpatch errors anymore (the one in visorhba_main 
was discussed before and decided to be okay). There are 7 files that 
generate warnings still, controlvmchannel.h being the biggest offender, 
with the rest all in single digits.

I'll get those warnings cleaned up - are there other things you'd like 
us to fix?

-- Ben

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

* Re: [PATCH 00/14] staging: unisys: add channel interrupt support
  2015-11-20 18:59   ` Ben Romer
@ 2015-11-20 21:51     ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2015-11-20 21:51 UTC (permalink / raw)
  To: Ben Romer; +Cc: sparmaintainer, driverdev-devel

On Fri, Nov 20, 2015 at 01:59:02PM -0500, Ben Romer wrote:
> On 11/17/2015 05:18 PM, Greg KH wrote:
> >On Tue, Nov 17, 2015 at 09:57:58AM -0500, Benjamin Romer wrote:
> >>This patch series adds a centralized infrastructure and device support
> >>for channel interrupts sent to s-Par virtual devices. With these changes,
> >>the visorhba device is ~80% faster than with only polling, and visornic
> >>receives a speedup of over 3500% (from ~9Mb/s to between 360Mb/s and
> >>390Mb/s).
> >
> >Why are you adding new functions to the code before it is merged out of
> >staging?  Please don't do that, please get the code out of staging
> >first.
> >
> >thanks,
> >
> >greg k-h
> >
> 
> Hi Greg,
> 
> Interrupt support was originally part of the driver set when we submitted
> it, but we removed it when we renamed the drivers and restructured the set.
> The work was listed in the TODO so I thought it would be okay to patch it in
> staging, but if we should wait until the drivers are out, then we will wait.
> :)

Well, it might be, but as you didn't say that in the changelog here, how
was I supposed to remember that?  :)

> Do you think our drivers are clean enough to be moved out soon? None of the
> files generate checkpatch errors anymore (the one in visorhba_main was
> discussed before and decided to be okay). There are 7 files that generate
> warnings still, controlvmchannel.h being the biggest offender, with the rest
> all in single digits.
> 
> I'll get those warnings cleaned up - are there other things you'd like us to
> fix?

That's a great first-step, after that, we can do an audit of the code,
but please do all of the "easy things" first.

thanks,

greg k-h

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

end of thread, other threads:[~2015-11-20 21:51 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-17 14:57 [PATCH 00/14] staging: unisys: add channel interrupt support Benjamin Romer
2015-11-17 14:57 ` [PATCH 01/14] staging: unisys: Change poll rate to 2 ms for work Benjamin Romer
2015-11-17 22:19   ` Greg KH
2015-11-17 14:58 ` [PATCH 02/14] staging: unisys: set client state Benjamin Romer
2015-11-17 14:58 ` [PATCH 03/14] staging: unisys: visorhba: Convert visorhba to use visorbus channel interrupts Benjamin Romer
2015-11-17 14:58 ` [PATCH 04/14] staging: unisys: Convert visornic to use visorbus channel interrupt code Benjamin Romer
2015-11-17 14:58 ` [PATCH 05/14] staging: unisys: visorinput: use spinlock for channel_interrupt() locking Benjamin Romer
2015-11-17 14:58 ` [PATCH 06/14] staging: unisys: Only process up to budget amount of responses Benjamin Romer
2015-11-17 14:58 ` [PATCH 07/14] staging: unisys: Add support to update Features bits in channel queues Benjamin Romer
2015-11-17 14:58 ` [PATCH 08/14] staging: unisys: Add channel feature access functions Benjamin Romer
2015-11-17 14:58 ` [PATCH 09/14] staging: unisys: Re-enable interrupts after we have done the work Benjamin Romer
2015-11-17 14:58 ` [PATCH 10/14] staging: unisys: Capture data from device create to register interrupt Benjamin Romer
2015-11-17 14:58 ` [PATCH 11/14] staging: unisys: Don't go into POLLING mode in visornic_probe Benjamin Romer
2015-11-17 14:58 ` [PATCH 12/14] staging: unisys: Don't set polling mode in visorhba_probe Benjamin Romer
2015-11-17 14:58 ` [PATCH 13/14] staging: unisys: Remove semaphores around channel interrupts Benjamin Romer
2015-11-17 14:58 ` [PATCH 14/14] staging: unisys: Allow for unregistering of interrupts Benjamin Romer
2015-11-17 22:18 ` [PATCH 00/14] staging: unisys: add channel interrupt support Greg KH
2015-11-20 18:59   ` Ben Romer
2015-11-20 21:51     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.