On Tue, 27 May 2008 19:05:26 +0900
Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> wrote:
> Fix improper long wait for command completion in pciehp probing.
>
> As described in PCI Express specification, software notification is
> not generated if the command that occurs as a result of a write to the
> Slot Control register that disables software notification of command
> completed events. Since pciehp driver doesn't take it into account,
> such command is issued in pciehp probing, and it causes improper long
> wait for command completion.
>
> This patch changes the pciehp driver to take such command into
> account.
>
> ...
>
> -static inline int pcie_wait_cmd(struct controller *ctrl)
> +static inline int pcie_poll_cmd(struct controller *ctrl)
> +{
> + u16 slot_status;
> + int timeout = 1000;
> +
> + if (!pciehp_readw(ctrl, SLOTSTATUS, &slot_status))
> + if (slot_status & CMD_COMPLETED)
> + goto completed;
> + for (timeout = 1000; timeout > 0; timeout -= 100) {
> + msleep(100);
> + if (!pciehp_readw(ctrl, SLOTSTATUS, &slot_status))
> + if (slot_status & CMD_COMPLETED)
> + goto completed;
> + }
> + return 0; /* timeout */
> +
> +completed:
> + pciehp_writew(ctrl, SLOTSTATUS, CMD_COMPLETED);
> + return timeout;
> +}
This looks a bit ungainly. Couldn't we do something along the lines of
static inline int pcie_poll_cmd(struct controller *ctrl)
{
int timeout = 1000;
while (timeout > 0) {
u16 slot_status;
if (!pciehp_readw(ctrl, SLOTSTATUS, &slot_status)) {
if (slot_status & CMD_COMPLETED) {
pciehp_writew(ctrl, SLOTSTATUS, CMD_COMPLETED);
break;
}
}
msleep(100);
timeout -= 100;
}
return timeout;
}
Also, I wonder if we wold get further improvements by polling at 1kHz
rather than at 10Hz. Or maybe at 100Hz, as 1kHz might be problematic
on HZ=100 kernels.
> +static inline int pcie_wait_cmd(struct controller *ctrl, int poll)
These functions are too large to be inlined.
> {
> int retval = 0;
> unsigned int msecs = pciehp_poll_mode ? 2500 : 1000;
> unsigned long timeout = msecs_to_jiffies(msecs);
> int rc;
>
> - rc = wait_event_interruptible_timeout(ctrl->queue,
> + if (poll)
> + rc = pcie_poll_cmd(ctrl);
> + else
> + rc = wait_event_interruptible_timeout(ctrl->queue,
> !ctrl->cmd_busy, timeout);
> if (!rc)
> dbg("Command not completed in 1000 msec\n");
> @@ -331,8 +355,18 @@ static int pcie_write_cmd(struct control
> /*
> * Wait for command completion.
> */
> - if (!retval && !ctrl->no_cmd_complete)
> - retval = pcie_wait_cmd(ctrl);
> + if (!retval && !ctrl->no_cmd_complete) {
> + int poll = 0;
> + /*
> + * if hotplug interrupt is not enabled or command
> + * completed interrupt is not enabled, we need to poll
> + * command completed event.
> + */
> + if (!(slot_ctrl & HP_INTR_ENABLE) ||
> + !(slot_ctrl & CMD_CMPL_INTR_ENABLE))
> + poll = 1;
> + retval = pcie_wait_cmd(ctrl, poll);
> + }
> out:
> mutex_unlock(&ctrl->ctrl_lock);
> return retval;
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
