pciehp: Fix interrupt event handlig

Current pciehp implementation disables and re-enables hotplug interrupts
in its interrupt handler. This operation might be intend to guarantee
that interrupts for the events newly occured during previous events are
being handled will be successfully generated. But current implementaion
has the following prolems.

  - Current interrupt service routin clears status changes without
    waiting command completion. Because of this, events might not be
    cleared properly.
  - Current interrupt service routine clears status changes caused by
    disabling or enabling hotplug interrupts itself. This will lose new
    events that occurs during previous interrupts are being handled.
  - Current implementation doesn't have any serialization mechanism
    between the code to wait for command completion and the interrupt
    handler that clears the command completion events caused by itself.
    There is clearly race conditions between them, and it may cause
    the problem that waiting for command completion doesn't work for
    example.

To fix those problems, this patch stops disabling/re-enabling hotplug
interrupts in interrupt service routine. Instead of this, this patch
re-inspects Slot Status register after clearing what is presumed to
be the last bending interrupt in order to guarantee that all interrupt
events are serviced.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
This commit is contained in:
Kenji Kaneshige 2008-04-25 14:38:57 -07:00 committed by Jesse Barnes
parent 3800345f72
commit c6b069e946
2 changed files with 30 additions and 121 deletions

View File

@ -97,7 +97,6 @@ struct controller {
u8 cap_base; u8 cap_base;
struct timer_list poll_timer; struct timer_list poll_timer;
volatile int cmd_busy; volatile int cmd_busy;
spinlock_t lock;
}; };
#define INT_BUTTON_IGNORE 0 #define INT_BUTTON_IGNORE 0

View File

@ -252,7 +252,6 @@ static int pcie_write_cmd(struct slot *slot, u16 cmd, u16 mask)
int retval = 0; int retval = 0;
u16 slot_status; u16 slot_status;
u16 slot_ctrl; u16 slot_ctrl;
unsigned long flags;
mutex_lock(&ctrl->ctrl_lock); mutex_lock(&ctrl->ctrl_lock);
@ -270,11 +269,10 @@ static int pcie_write_cmd(struct slot *slot, u16 cmd, u16 mask)
__func__); __func__);
} }
spin_lock_irqsave(&ctrl->lock, flags);
retval = pciehp_readw(ctrl, SLOTCTRL, &slot_ctrl); retval = pciehp_readw(ctrl, SLOTCTRL, &slot_ctrl);
if (retval) { if (retval) {
err("%s: Cannot read SLOTCTRL register\n", __func__); err("%s: Cannot read SLOTCTRL register\n", __func__);
goto out_spin_unlock; goto out;
} }
slot_ctrl &= ~mask; slot_ctrl &= ~mask;
@ -285,9 +283,6 @@ static int pcie_write_cmd(struct slot *slot, u16 cmd, u16 mask)
if (retval) if (retval)
err("%s: Cannot write to SLOTCTRL register\n", __func__); err("%s: Cannot write to SLOTCTRL register\n", __func__);
out_spin_unlock:
spin_unlock_irqrestore(&ctrl->lock, flags);
/* /*
* Wait for command completion. * Wait for command completion.
*/ */
@ -733,139 +728,55 @@ static int hpc_power_off_slot(struct slot * slot)
static irqreturn_t pcie_isr(int irq, void *dev_id) static irqreturn_t pcie_isr(int irq, void *dev_id)
{ {
struct controller *ctrl = (struct controller *)dev_id; struct controller *ctrl = (struct controller *)dev_id;
u16 slot_status, intr_detect, intr_loc; u16 detected, intr_loc;
u16 temp_word;
int hp_slot = 0; /* only 1 slot per PCI Express port */
int rc = 0;
unsigned long flags;
rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status); /*
if (rc) { * In order to guarantee that all interrupt events are
err("%s: Cannot read SLOTSTATUS register\n", __func__); * serviced, we need to re-inspect Slot Status register after
return IRQ_NONE; * clearing what is presumed to be the last pending interrupt.
} */
intr_loc = 0;
intr_detect = (ATTN_BUTTN_PRESSED | PWR_FAULT_DETECTED | do {
MRL_SENS_CHANGED | PRSN_DETECT_CHANGED | CMD_COMPLETED); if (pciehp_readw(ctrl, SLOTSTATUS, &detected)) {
err("%s: Cannot read SLOTSTATUS\n", __func__);
intr_loc = slot_status & intr_detect;
/* Check to see if it was our interrupt */
if ( !intr_loc )
return IRQ_NONE;
dbg("%s: intr_loc %x\n", __func__, intr_loc);
/* Mask Hot-plug Interrupt Enable */
if (!pciehp_poll_mode) {
spin_lock_irqsave(&ctrl->lock, flags);
rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
if (rc) {
err("%s: Cannot read SLOT_CTRL register\n",
__func__);
spin_unlock_irqrestore(&ctrl->lock, flags);
return IRQ_NONE; return IRQ_NONE;
} }
dbg("%s: pciehp_readw(SLOTCTRL) with value %x\n", detected &= (ATTN_BUTTN_PRESSED | PWR_FAULT_DETECTED |
__func__, temp_word); MRL_SENS_CHANGED | PRSN_DETECT_CHANGED |
temp_word = (temp_word & ~HP_INTR_ENABLE & CMD_COMPLETED);
~CMD_CMPL_INTR_ENABLE) | 0x00; intr_loc |= detected;
rc = pciehp_writew(ctrl, SLOTCTRL, temp_word); if (!intr_loc)
if (rc) { return IRQ_NONE;
err("%s: Cannot write to SLOTCTRL register\n", if (pciehp_writew(ctrl, SLOTSTATUS, detected)) {
__func__); err("%s: Cannot write to SLOTSTATUS\n", __func__);
spin_unlock_irqrestore(&ctrl->lock, flags);
return IRQ_NONE; return IRQ_NONE;
} }
spin_unlock_irqrestore(&ctrl->lock, flags); } while (detected);
rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status); dbg("%s: intr_loc %x\n", __FUNCTION__, intr_loc);
if (rc) {
err("%s: Cannot read SLOT_STATUS register\n",
__func__);
return IRQ_NONE;
}
dbg("%s: pciehp_readw(SLOTSTATUS) with value %x\n",
__func__, slot_status);
/* Clear command complete interrupt caused by this write */
temp_word = 0x1f;
rc = pciehp_writew(ctrl, SLOTSTATUS, temp_word);
if (rc) {
err("%s: Cannot write to SLOTSTATUS register\n",
__func__);
return IRQ_NONE;
}
}
/* Check Command Complete Interrupt Pending */
if (intr_loc & CMD_COMPLETED) { if (intr_loc & CMD_COMPLETED) {
/*
* Command Complete Interrupt Pending
*/
ctrl->cmd_busy = 0; ctrl->cmd_busy = 0;
wake_up_interruptible(&ctrl->queue); wake_up_interruptible(&ctrl->queue);
} }
/* Check MRL Sensor Changed */
if (intr_loc & MRL_SENS_CHANGED) if (intr_loc & MRL_SENS_CHANGED)
pciehp_handle_switch_change(hp_slot, ctrl); pciehp_handle_switch_change(0, ctrl);
/* Check Attention Button Pressed */
if (intr_loc & ATTN_BUTTN_PRESSED) if (intr_loc & ATTN_BUTTN_PRESSED)
pciehp_handle_attention_button(hp_slot, ctrl); pciehp_handle_attention_button(0, ctrl);
/* Check Presence Detect Changed */
if (intr_loc & PRSN_DETECT_CHANGED) if (intr_loc & PRSN_DETECT_CHANGED)
pciehp_handle_presence_change(hp_slot, ctrl); pciehp_handle_presence_change(0, ctrl);
/* Check Power Fault Detected */
if (intr_loc & PWR_FAULT_DETECTED) if (intr_loc & PWR_FAULT_DETECTED)
pciehp_handle_power_fault(hp_slot, ctrl); pciehp_handle_power_fault(0, ctrl);
/* Clear all events after serving them */
temp_word = 0x1F;
rc = pciehp_writew(ctrl, SLOTSTATUS, temp_word);
if (rc) {
err("%s: Cannot write to SLOTSTATUS register\n", __func__);
return IRQ_NONE;
}
/* Unmask Hot-plug Interrupt Enable */
if (!pciehp_poll_mode) {
spin_lock_irqsave(&ctrl->lock, flags);
rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
if (rc) {
err("%s: Cannot read SLOTCTRL register\n",
__func__);
spin_unlock_irqrestore(&ctrl->lock, flags);
return IRQ_NONE;
}
dbg("%s: Unmask Hot-plug Interrupt Enable\n", __func__);
temp_word = (temp_word & ~HP_INTR_ENABLE) | HP_INTR_ENABLE;
rc = pciehp_writew(ctrl, SLOTCTRL, temp_word);
if (rc) {
err("%s: Cannot write to SLOTCTRL register\n",
__func__);
spin_unlock_irqrestore(&ctrl->lock, flags);
return IRQ_NONE;
}
spin_unlock_irqrestore(&ctrl->lock, flags);
rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status);
if (rc) {
err("%s: Cannot read SLOT_STATUS register\n",
__func__);
return IRQ_NONE;
}
/* Clear command complete interrupt caused by this write */
temp_word = 0x1F;
rc = pciehp_writew(ctrl, SLOTSTATUS, temp_word);
if (rc) {
err("%s: Cannot write to SLOTSTATUS failed\n",
__func__);
return IRQ_NONE;
}
dbg("%s: pciehp_writew(SLOTSTATUS) with value %x\n",
__func__, temp_word);
}
return IRQ_HANDLED; return IRQ_HANDLED;
} }
@ -1334,7 +1245,6 @@ int pcie_init(struct controller *ctrl, struct pcie_device *dev)
mutex_init(&ctrl->crit_sect); mutex_init(&ctrl->crit_sect);
mutex_init(&ctrl->ctrl_lock); mutex_init(&ctrl->ctrl_lock);
spin_lock_init(&ctrl->lock);
/* setup wait queue */ /* setup wait queue */
init_waitqueue_head(&ctrl->queue); init_waitqueue_head(&ctrl->queue);