[PATCH] shpchp: fix improper wait for command completion
Current SHPCHP driver uses msleep_interruptible() function to wait for
a command completion event. But I think this would cause an unnecessary
long wait until timeout, if command completion interrupt came before
task state was changed to TASK_INTERRUPTIBLE. This patch fixes this
issue. With this patch, command completion becomes faster as follows:
o Without this patch
# time echo 1 > power
real 0m4.708s
user 0m0.000s
sys 0m0.524s
o With this patch
# time echo 1 > power
real 0m2.221s
user 0m0.000s
sys 0m0.532s
Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h
index 55b0cd1..ce0e9b6 100644
--- a/drivers/pci/hotplug/shpchp.h
+++ b/drivers/pci/hotplug/shpchp.h
@@ -101,6 +101,7 @@
u32 cap_offset;
unsigned long mmio_base;
unsigned long mmio_size;
+ volatile int cmd_busy;
};
struct hotplug_params {
diff --git a/drivers/pci/hotplug/shpchp_ctrl.c b/drivers/pci/hotplug/shpchp_ctrl.c
index 5861935..25ccb0e 100644
--- a/drivers/pci/hotplug/shpchp_ctrl.c
+++ b/drivers/pci/hotplug/shpchp_ctrl.c
@@ -248,7 +248,6 @@
up(&ctrl->crit_sect);
return WRONG_BUS_FREQUENCY;
}
- wait_for_ctrl_irq (ctrl);
if ((rc = p_slot->hpc_ops->check_cmd_status(ctrl))) {
err("%s: Can't set bus speed/mode in the case of adapter & bus mismatch\n",
@@ -330,9 +329,6 @@
up(&ctrl->crit_sect);
return -1;
}
-
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
rc = p_slot->hpc_ops->check_cmd_status(ctrl);
if (rc) {
@@ -352,7 +348,6 @@
up(&ctrl->crit_sect);
return WRONG_BUS_FREQUENCY;
}
- wait_for_ctrl_irq (ctrl);
if ((rc = p_slot->hpc_ops->check_cmd_status(ctrl))) {
err("%s: Can't set bus speed/mode in the case of adapter & bus mismatch\n",
@@ -367,7 +362,6 @@
up(&ctrl->crit_sect);
return rc;
}
- wait_for_ctrl_irq (ctrl);
if ((rc = p_slot->hpc_ops->check_cmd_status(ctrl))) {
err("%s: Failed to enable slot, error code(%d)\n", __FUNCTION__, rc);
@@ -494,7 +488,6 @@
up(&ctrl->crit_sect);
return rc;
}
- wait_for_ctrl_irq (ctrl);
if ((rc = p_slot->hpc_ops->check_cmd_status(ctrl))) {
err("%s: Failed to enable slot, error code(%d)\n", __FUNCTION__, rc);
@@ -532,9 +525,6 @@
p_slot->hpc_ops->green_led_on(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
-
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
@@ -552,8 +542,6 @@
up(&ctrl->crit_sect);
return rc;
}
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
rc = p_slot->hpc_ops->check_cmd_status(ctrl);
if (rc) {
@@ -603,8 +591,6 @@
up(&ctrl->crit_sect);
return rc;
}
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
rc = p_slot->hpc_ops->check_cmd_status(ctrl);
if (rc) {
@@ -621,8 +607,6 @@
up(&ctrl->crit_sect);
return rc;
}
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
@@ -676,9 +660,6 @@
p_slot->hpc_ops->green_led_off(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (p_slot->ctrl);
-
/* Done with exclusive hardware access */
up(&p_slot->ctrl->crit_sect);
}
@@ -790,14 +771,9 @@
down(&ctrl->crit_sect);
p_slot->hpc_ops->green_led_on(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
p_slot->hpc_ops->set_attention_status(p_slot, 0);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
-
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
break;
@@ -806,12 +782,8 @@
down(&ctrl->crit_sect);
p_slot->hpc_ops->green_led_off(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
p_slot->hpc_ops->set_attention_status(p_slot, 0);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
@@ -845,14 +817,9 @@
/* blink green LED and turn off amber */
p_slot->hpc_ops->green_led_blink(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
p_slot->hpc_ops->set_attention_status(p_slot, 0);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
-
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
@@ -870,12 +837,8 @@
down(&ctrl->crit_sect);
p_slot->hpc_ops->set_attention_status(p_slot, 1);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
p_slot->hpc_ops->green_led_off(p_slot);
- /* Wait for the command to complete */
- wait_for_ctrl_irq (ctrl);
/* Done with exclusive hardware access */
up(&ctrl->crit_sect);
diff --git a/drivers/pci/hotplug/shpchp_hpc.c b/drivers/pci/hotplug/shpchp_hpc.c
index f25e116..b4226ff 100644
--- a/drivers/pci/hotplug/shpchp_hpc.c
+++ b/drivers/pci/hotplug/shpchp_hpc.c
@@ -275,6 +275,25 @@
return;
}
+static inline int shpc_wait_cmd(struct controller *ctrl)
+{
+ int retval = 0;
+ unsigned int timeout_msec = shpchp_poll_mode ? 2000 : 1000;
+ unsigned long timeout = msecs_to_jiffies(timeout_msec);
+ int rc = wait_event_interruptible_timeout(ctrl->queue,
+ !ctrl->cmd_busy, timeout);
+ if (!rc) {
+ retval = -EIO;
+ err("Command not completed in %d msec\n", timeout_msec);
+ } else if (rc < 0) {
+ retval = -EINTR;
+ info("Command was interrupted by a signal\n");
+ }
+ ctrl->cmd_busy = 0;
+
+ return retval;
+}
+
static int shpc_write_cmd(struct slot *slot, u8 t_slot, u8 cmd)
{
struct php_ctlr_state_s *php_ctlr = slot->ctrl->hpc_ctlr_handle;
@@ -314,8 +333,14 @@
/* To make sure the Controller Busy bit is 0 before we send out the
* command.
*/
+ slot->ctrl->cmd_busy = 1;
writew(temp_word, php_ctlr->creg + CMD);
+ /*
+ * Wait for command completion.
+ */
+ retval = shpc_wait_cmd(slot->ctrl);
+
DBG_LEAVE_ROUTINE
return retval;
}
@@ -1064,6 +1089,7 @@
temp_dword = readl(php_ctlr->creg + SERR_INTR_ENABLE);
temp_dword &= 0xfffdffff;
writel(temp_dword, php_ctlr->creg + SERR_INTR_ENABLE);
+ ctrl->cmd_busy = 0;
wake_up_interruptible(&ctrl->queue);
}