Message-ID: <1465420463-24982-24-git-send-email-david.kershner@unisys.com>
Date:	Wed, 8 Jun 2016 17:14:17 -0400
From:	David Kershner <david.kershner@...sys.com>
To:	<corbet@....net>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<hpa@...or.com>, <david.kershner@...sys.com>,
	<gregkh@...uxfoundation.org>, <erik.arfvidson@...sys.com>,
	<timothy.sell@...sys.com>, <hofrat@...dl.org>,
	<dzickus@...hat.com>, <jes.sorensen@...hat.com>,
	<alexander.curtin@...sys.com>, <janani.rvchndrn@...il.com>,
	<sudipm.mukherjee@...il.com>, <prarit@...hat.com>,
	<david.binder@...sys.com>, <nhorman@...hat.com>,
	<dan.j.williams@...el.com>, <linux-kernel@...r.kernel.org>,
	<linux-doc@...r.kernel.org>,
	<driverdev-devel@...uxdriverproject.org>,
	<sparmaintainer@...sys.com>
Subject: [PATCH v4 23/29] staging: unisys: visorbus: Remove notifier-related code from visorbus

From: David Binder <david.binder@...sys.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@...sys.com>
Signed-off-by: David Kershner <david.kershner@...sys.com>
Reviewed-by: Tim Sell <timothy.sell@...sys.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

Powered by blists - more mailing lists