Andrew Morton wrote:
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;
}
Thank you very much for review.
I will clean it up based on your suggestion.
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.
I will change the frequency to 100Hz.
In addition to your comments, I think maybe 1kHz is too frequent because
PCI Express specification allows relatively long time (maximum 1 sec.)
for command completion.
+static inline int pcie_wait_cmd(struct controller *ctrl, int poll)
These functions are too large to be inlined.
I will remove those 'inline's.
BTW, Jesse-san already sent git pull request. So I wonder I should
make above changes as incremental patches. Is it ok, Jesse-san? Or
should I update the original ones?
Thanks,
Kenji Kaneshige
--
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
