Hi Yinghai, My understanding of PCIe spec is that a write transaction to any portion of the Slot Control register needs to be handled as a command. So, the behavior of your hotplug capable port seems to be against the PCIe spec. Any comments on this from your HW engineer? The Command Completed bit in the Slot Status Register indicates that the controller is ready to accept the subsequent command. With your change, pciehp driver can issue a command before the Command Completed bit is set on the controller that handles a write transaction to any portion of the Slot Control register as a command. That is, pciehp can issue a command before the controller becomes ready to accept the subsequent command. I think this would cause something wrong (PCIe spec says this is considered a programming error). So I think another approach is required. Thanks, Kenji Kaneshige (2010/06/02 3:49), Yinghai Lu wrote:
Found one system with PCIe Modules keep saying "Command not completed in 1000 msec" when pciehp initialized, and hot-add and hot-remove. According to our HW engineer Jason Timpe, Command Completed bit is set iff those real commands are issued. Because those command that are with VPP are going through i2c to vpp. So try to check that before we decide to wait those command completion. Signed-off-by: Yinghai Lu<yinghai@xxxxxxxxxx> --- drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) Index: linux-2.6/drivers/pci/hotplug/pciehp_hpc.c =================================================================== --- linux-2.6.orig/drivers/pci/hotplug/pciehp_hpc.c +++ linux-2.6/drivers/pci/hotplug/pciehp_hpc.c @@ -176,6 +176,7 @@ static int pcie_write_cmd(struct control int retval = 0; u16 slot_status; u16 slot_ctrl; + u16 wait_mask; mutex_lock(&ctrl->ctrl_lock); @@ -215,9 +216,14 @@ static int pcie_write_cmd(struct control goto out; } + wait_mask = mask& (PCI_EXP_SLTCTL_AIC | PCI_EXP_SLTCTL_PIC | PCI_EXP_SLTCTL_PCC); + if (wait_mask&& ((slot_ctrl& wait_mask) == (cmd& wait_mask))) + wait_mask = 0; + if (wait_mask) + ctrl->cmd_busy = 1; + slot_ctrl&= ~mask; slot_ctrl |= (cmd& mask); - ctrl->cmd_busy = 1; smp_mb(); retval = pciehp_writew(ctrl, PCI_EXP_SLTCTL, slot_ctrl); if (retval) @@ -226,7 +232,7 @@ static int pcie_write_cmd(struct control /* * Wait for command completion. */ - if (!retval&& !ctrl->no_cmd_complete) { + if (!retval&& !ctrl->no_cmd_complete&& wait_mask) { int poll = 0; /* * if hotplug interrupt is not enabled or command -- 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
-- 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