[PATCH] shpchp: Fix slot state handling

Current SHPCHP driver doesn't care about the confliction between
hotplug operation via sysfs and hotplug operation via attention
button. So if those ware conflicted, slot could be an unexpected
state.

This patch changes SHPCHP driver to handle slot state properly. With
this patch, slot events are handled according to the current slot
state as shown at the Table below.

		Table. Slot States and Event Handling
=========================================================================
Slot State		Event and Action
=========================================================================
STATIC			- Go to POWERON state if user initiates
(Slot enabled,		  insertion request via sysfs
 Slot disabled)		- Go to POWEROFF state if user initiates removal
			  request via sysfs
			- Go to BLINKINGON state if user presses
			  attention button when the slot is disabled
			- Go to BLINKINGOFF state if user presses
			  attention button when the slot is enabled

3 files changed, 181 insertions, 61 deletions

diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h
index 87db07c..dd44951 100644
--- a/drivers/pci/hotplug/shpchp.h
+++ b/drivers/pci/hotplug/shpchp.h
@@ -72,6 +72,7 @@ struct slot {
 	struct list_head	slot_list;
 	char name[SLOT_NAME_SIZE];
 	struct work_struct work;	/* work for button event */
+	struct mutex lock;
 };
 
 struct event_info {
@@ -181,8 +182,8 @@ struct hotplug_params {
 /* sysfs functions for the hotplug controller info */
 extern void shpchp_create_ctrl_files	(struct controller *ctrl);
 
-extern int	shpchp_enable_slot(struct slot *slot);
-extern int	shpchp_disable_slot(struct slot *slot);
+extern int	shpchp_sysfs_enable_slot(struct slot *slot);
+extern int	shpchp_sysfs_disable_slot(struct slot *slot);
 
 extern u8	shpchp_handle_attention_button(u8 hp_slot, void *inst_id);
 extern u8	shpchp_handle_switch_change(u8 hp_slot, void *inst_id);
@@ -200,7 +201,7 @@ extern int	shpchprm_get_physical_slot_number(struct controller *ctrl,
 		u32 *sun, u8 busnum, u8 devnum);
 extern void	shpchp_remove_ctrl_files(struct controller *ctrl);
 extern void	cleanup_slots(struct controller *ctrl);
-extern void	shpchp_pushbutton_thread(void *data);
+extern void	queue_pushbutton_work(void *data);
 
 /* Global variables */
 extern struct list_head shpchp_ctrl_list;
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 5de659d..fa60ae4 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -136,13 +136,14 @@ static int init_slots(struct controller *ctrl)
 		slot->bus = ctrl->slot_bus;
 		slot->device = ctrl->slot_device_offset + i;
 		slot->hpc_ops = ctrl->hpc_ops;
+		mutex_init(&slot->lock);
 
 		if (shpchprm_get_physical_slot_number(ctrl, &sun,
 						      slot->bus, slot->device))
 			goto error_info;
 
 		slot->number = sun;
-		INIT_WORK(&slot->work, shpchp_pushbutton_thread, slot);
+		INIT_WORK(&slot->work, queue_pushbutton_work, slot);
 
 		/* register this slot with the hotplug pci core */
 		hotplug_slot->private = slot;
@@ -188,6 +189,7 @@ void cleanup_slots(struct controller *ctrl)
 		slot = list_entry(tmp, struct slot, slot_list);
 		list_del(&slot->slot_list);
 		cancel_delayed_work(&slot->work);
+		flush_scheduled_work();
 		flush_workqueue(shpchp_wq);
 		pci_hp_deregister(slot->hotplug_slot);
 	}
@@ -244,7 +246,7 @@ static int enable_slot (struct hotplug_slot *hotplug_slot)
 
 	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);
 
-	return shpchp_enable_slot(slot);
+	return shpchp_sysfs_enable_slot(slot);
 }
 
 static int disable_slot (struct hotplug_slot *hotplug_slot)
@@ -253,7 +255,7 @@ static int disable_slot (struct hotplug_slot *hotplug_slot)
 
 	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);
 
-	return shpchp_disable_slot(slot);
+	return shpchp_sysfs_disable_slot(slot);
 }
 
 static int get_power_status (struct hotplug_slot *hotplug_slot, u8 *value)
diff --git a/drivers/pci/hotplug/shpchp_ctrl.c b/drivers/pci/hotplug/shpchp_ctrl.c
index 2411f3b..10f3257 100644
--- a/drivers/pci/hotplug/shpchp_ctrl.c
+++ b/drivers/pci/hotplug/shpchp_ctrl.c
@@ -37,6 +37,8 @@
 #include "shpchp.h"
 
 static void interrupt_event_handler(void *data);
+static int shpchp_enable_slot(struct slot *p_slot);
+static int shpchp_disable_slot(struct slot *p_slot);
 
 static int queue_interrupt_event(struct slot *p_slot, u32 event_type)
 {
@@ -50,7 +52,7 @@ static int queue_interrupt_event(struct slot *p_slot, u32 event_type)
 	info->p_slot = p_slot;
 	INIT_WORK(&info->work, interrupt_event_handler, info);
 
-	queue_work(shpchp_wq, &info->work);
+	schedule_work(&info->work);
 
 	return 0;
 }
@@ -73,24 +75,6 @@ u8 shpchp_handle_attention_button(u8 hp_slot, void *inst_id)
 	info("Button pressed on Slot(%d)\n", ctrl->first_slot + hp_slot);
 	event_type = INT_BUTTON_PRESS;
 
-	if ((p_slot->state == BLINKINGON_STATE)
-	    || (p_slot->state == BLINKINGOFF_STATE)) {
-		/* Cancel if we are still blinking; this means that we press the
-		 * attention again before the 5 sec. limit expires to cancel hot-add
-		 * or hot-remove
-		 */
-		event_type = INT_BUTTON_CANCEL;
-		info("Button cancel on Slot(%d)\n", ctrl->first_slot + hp_slot);
-	} else if ((p_slot->state == POWERON_STATE)
-		   || (p_slot->state == POWEROFF_STATE)) {
-		/* Ignore if the slot is on power-on or power-off state; this 
-		 * means that the previous attention button action to hot-add or
-		 * hot-remove is undergoing
-		 */
-		event_type = INT_BUTTON_IGNORE;
-		info("Button ignore on Slot(%d)\n", ctrl->first_slot + hp_slot);
-	}
-
 	queue_interrupt_event(p_slot, event_type);
 
 	return 0;
@@ -492,6 +476,11 @@ static int remove_board(struct slot *p_slot)
 }
 
 
+struct pushbutton_work_info {
+	struct slot *p_slot;
+	struct work_struct work;
+};
+
 /**
  * shpchp_pushbutton_thread
  *
@@ -499,22 +488,61 @@ static int remove_board(struct slot *p_slot)
  * Handles all pending events and exits.
  *
  */
-void shpchp_pushbutton_thread(void *data)
+static void shpchp_pushbutton_thread(void *data)
 {
-	struct slot *p_slot = data;
-	u8 getstatus;
+	struct pushbutton_work_info *info = data;
+	struct slot *p_slot = info->p_slot;
 
-	p_slot->hpc_ops->get_power_status(p_slot, &getstatus);
-	if (getstatus) {
-		p_slot->state = POWEROFF_STATE;
+	mutex_lock(&p_slot->lock);
+	switch (p_slot->state) {
+	case POWEROFF_STATE:
+		mutex_unlock(&p_slot->lock);
 		shpchp_disable_slot(p_slot);
+		mutex_lock(&p_slot->lock);
 		p_slot->state = STATIC_STATE;
-	} else {
-		p_slot->state = POWERON_STATE;
+		break;
+	case POWERON_STATE:
+		mutex_unlock(&p_slot->lock);
 		if (shpchp_enable_slot(p_slot))
 			p_slot->hpc_ops->green_led_off(p_slot);
+		mutex_lock(&p_slot->lock);
 		p_slot->state = STATIC_STATE;
+		break;
+	default:
+		break;
+	}
+	mutex_unlock(&p_slot->lock);
+
+	kfree(info);
+}
+
+void queue_pushbutton_work(void *data)
+{
+	struct slot *p_slot = data;
+	struct pushbutton_work_info *info;
+
+	info = kmalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		err("%s: Cannot allocate memory\n", __FUNCTION__);
+		return;
+	}
+	info->p_slot = p_slot;
+	INIT_WORK(&info->work, shpchp_pushbutton_thread, info);
+
+	mutex_lock(&p_slot->lock);
+	switch (p_slot->state) {
+	case BLINKINGOFF_STATE:
+		p_slot->state = POWEROFF_STATE;
+		break;
+	case BLINKINGON_STATE:
+		p_slot->state = POWERON_STATE;
+		break;
+	default:
+		goto out;
 	}
+	queue_work(shpchp_wq, &info->work);
+ out:
+	mutex_unlock(&p_slot->lock);
 }
 
 static int update_slot_info (struct slot *slot)
@@ -536,34 +564,15 @@ static int update_slot_info (struct slot *slot)
 	return result;
 }
 
-static void interrupt_event_handler(void *data)
+/*
+ * Note: This function must be called with slot->lock held
+ */
+static void handle_button_press_event(struct slot *p_slot)
 {
-	struct event_info *info = data;
-	struct slot *p_slot = info->p_slot;
 	u8 getstatus;
 
-	switch (info->event_type) {
-	case INT_BUTTON_CANCEL:
-		dbg("%s: button cancel\n", __FUNCTION__);
-		cancel_delayed_work(&p_slot->work);
-		switch (p_slot->state) {
-		case BLINKINGOFF_STATE:
-			p_slot->hpc_ops->green_led_on(p_slot);
-			p_slot->hpc_ops->set_attention_status(p_slot, 0);
-			break;
-		case BLINKINGON_STATE:
-			p_slot->hpc_ops->green_led_off(p_slot);
-			p_slot->hpc_ops->set_attention_status(p_slot, 0);
-			break;
-		default:
-			warn("Not a valid state\n");
-			return;
-		}
-		info(msg_button_cancel, p_slot->number);
-		p_slot->state = STATIC_STATE;
-		break;
-	case INT_BUTTON_PRESS:
-		dbg("%s: Button pressed\n", __FUNCTION__);
+	switch (p_slot->state) {
+	case STATIC_STATE:
 		p_slot->hpc_ops->get_power_status(p_slot, &getstatus);
 		if (getstatus) {
 			p_slot->state = BLINKINGOFF_STATE;
@@ -576,7 +585,51 @@ static void interrupt_event_handler(void *data)
 		p_slot->hpc_ops->green_led_blink(p_slot);
 		p_slot->hpc_ops->set_attention_status(p_slot, 0);
 
-		queue_delayed_work(shpchp_wq, &p_slot->work, 5*HZ);
+		schedule_delayed_work(&p_slot->work, 5*HZ);
+		break;
+	case BLINKINGOFF_STATE:
+	case BLINKINGON_STATE:
+		/*
+		 * Cancel if we are still blinking; this means that we
+		 * press the attention again before the 5 sec. limit
+		 * expires to cancel hot-add or hot-remove
+		 */
+		info("Button cancel on Slot(%s)\n", p_slot->name);
+		dbg("%s: button cancel\n", __FUNCTION__);
+		cancel_delayed_work(&p_slot->work);
+		if (p_slot->state == BLINKINGOFF_STATE)
+			p_slot->hpc_ops->green_led_on(p_slot);
+		else
+			p_slot->hpc_ops->green_led_off(p_slot);
+		p_slot->hpc_ops->set_attention_status(p_slot, 0);
+		info(msg_button_cancel, p_slot->number);
+		p_slot->state = STATIC_STATE;
+		break;
+	case POWEROFF_STATE:
+	case POWERON_STATE:
+		/*
+		 * Ignore if the slot is on power-on or power-off state;
+		 * this means that the previous attention button action
+		 * to hot-add or hot-remove is undergoing
+		 */
+		info("Button ignore on Slot(%s)\n", p_slot->name);
+		update_slot_info(p_slot);
+		break;
+	default:
+		warn("Not a valid state\n");
+		break;
+	}
+}
+
+static void interrupt_event_handler(void *data)
+{
+	struct event_info *info = data;
+	struct slot *p_slot = info->p_slot;
+
+	mutex_lock(&p_slot->lock);
+	switch (info->event_type) {
+	case INT_BUTTON_PRESS:
+		handle_button_press_event(p_slot);
 		break;
 	case INT_POWER_FAULT:
 		dbg("%s: power fault\n", __FUNCTION__);
@@ -587,12 +640,13 @@ static void interrupt_event_handler(void *data)
 		update_slot_info(p_slot);
 		break;
 	}
+	mutex_unlock(&p_slot->lock);
 
 	kfree(info);
 }
 
 
-int shpchp_enable_slot (struct slot *p_slot)
+static int shpchp_enable_slot (struct slot *p_slot)
 {
 	u8 getstatus = 0;
 	int rc, retval = -ENODEV;
@@ -647,7 +701,7 @@ int shpchp_enable_slot (struct slot *p_slot)
 }
 
 
-int shpchp_disable_slot (struct slot *p_slot)
+static int shpchp_disable_slot (struct slot *p_slot)
 {
 	u8 getstatus = 0;
 	int rc, retval = -ENODEV;
@@ -681,3 +735,66 @@ int shpchp_disable_slot (struct slot *p_slot)
 	return retval;
 }
 
+int shpchp_sysfs_enable_slot(struct slot *p_slot)
+{
+	int retval = -ENODEV;
+
+	mutex_lock(&p_slot->lock);
+	switch (p_slot->state) {
+	case BLINKINGON_STATE:
+		cancel_delayed_work(&p_slot->work);
+	case STATIC_STATE:
+		p_slot->state = POWERON_STATE;
+		mutex_unlock(&p_slot->lock);
+		retval = shpchp_enable_slot(p_slot);
+		mutex_lock(&p_slot->lock);
+		p_slot->state = STATIC_STATE;
+		break;
+	case POWERON_STATE:
+		info("Slot %s is already in powering on state\n",
+		     p_slot->name);
+		break;
+	case BLINKINGOFF_STATE:
+	case POWEROFF_STATE:
+		info("Already enabled on slot %s\n", p_slot->name);
+		break;
+	default:
+		err("Not a valid state on slot %s\n", p_slot->name);
+		break;
+	}
+	mutex_unlock(&p_slot->lock);
+
+	return retval;
+}
+
+int shpchp_sysfs_disable_slot(struct slot *p_slot)
+{
+	int retval = -ENODEV;
+
+	mutex_lock(&p_slot->lock);
+	switch (p_slot->state) {
+	case BLINKINGOFF_STATE:
+		cancel_delayed_work(&p_slot->work);
+	case STATIC_STATE:
+		p_slot->state = POWEROFF_STATE;
+		mutex_unlock(&p_slot->lock);
+		retval = shpchp_disable_slot(p_slot);
+		mutex_lock(&p_slot->lock);
+		p_slot->state = STATIC_STATE;
+		break;
+	case POWEROFF_STATE:
+		info("Slot %s is already in powering off state\n",
+		     p_slot->name);
+		break;
+	case BLINKINGON_STATE:
+	case POWERON_STATE:
+		info("Already disabled on slot %s\n", p_slot->name);
+		break;
+	default:
+		err("Not a valid state on slot %s\n", p_slot->name);
+		break;
+	}
+	mutex_unlock(&p_slot->lock);
+
+	return retval;
+}