Re: [RFC PATCH] pciehp: only wait command complete for real command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux