Re: [patch] PCI: pciehp: Wait for 1 second in board_added() for a Valid Configuration Request

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

 



(2011/07/05 17:01), Naoki Yanagimoto wrote:
> From: Naoki Yanagimoto<yanagimoto@xxxxxxxxxxxxxxxxxx>
> 
> I got a problem that an abnormal value was returned from the configuration
> space of some PCIe card at hotadd. The pciehp driver regarded the function of
> the device as being unavailable, so the card did not work. The problem
> disappeared when I simply added 1 second wait without using DLLLA.
> 
> I think that it should wait for 1 second because "PCI Express Base
> Specification Revision 3.0" says, "the software must wait for at least
> 1 second to judge device is broken after Data Link Layer State Changed Event".
> 

I think you are right.

> Therefore, I send a patch that drops DLLLA checking and adds 1 second wait.

But I think DLLLA checking is still required. I think we need the following things.

- Wait for Data Link Layer State Changed event.
- Wait for at least 1 second to judge device is broken after Data Link
  Layer State Changed Event

Your patch does only the latter. I think we also need the former.
What do you think about the following change? Could you try it?
Please note that I've not tested it at all. Sorry...

Regards,
Kenji Kaneshige

---
 drivers/pci/hotplug/pciehp_ctrl.c |    3 +++
 drivers/pci/hotplug/pciehp_hpc.c  |   11 ++---------
 2 files changed, 5 insertions(+), 9 deletions(-)

Index: 20110705/drivers/pci/hotplug/pciehp_ctrl.c
===================================================================
--- 20110705.orig/drivers/pci/hotplug/pciehp_ctrl.c
+++ 20110705/drivers/pci/hotplug/pciehp_ctrl.c
@@ -213,6 +213,9 @@ static int board_added(struct slot *p_sl
                goto err_exit;
        }

+       /* Wait for 1 second after checking link training status */
+       msleep(1000);
+
        /* Check for a power fault */
        if (ctrl->power_fault_detected || pciehp_query_power_fault(p_slot)) {
                ctrl_err(ctrl, "Power fault on slot %s\n", slot_name(p_slot));
Index: 20110705/drivers/pci/hotplug/pciehp_hpc.c
===================================================================
--- 20110705.orig/drivers/pci/hotplug/pciehp_hpc.c
+++ 20110705/drivers/pci/hotplug/pciehp_hpc.c
@@ -275,16 +275,9 @@ int pciehp_check_link_status(struct cont
          * hot-plug capable downstream port. But old controller might
          * not implement it. In this case, we wait for 1000 ms.
          */
-        if (ctrl->link_active_reporting){
-                /* Wait for Data Link Layer Link Active bit to be set */
+        if (ctrl->link_active_reporting)
                 pcie_wait_link_active(ctrl);
-                /*
-                 * We must wait for 100 ms after the Data Link Layer
-                 * Link Active bit reads 1b before initiating a
-                 * configuration access to the hot added device.
-                 */
-                msleep(100);
-        } else
+        else
                 msleep(1000);

        retval = pciehp_readw(ctrl, PCI_EXP_LNKSTA, &lnk_status);

--
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