From patchwork Tue Jul 5 08:43:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kenji Kaneshige X-Patchwork-Id: 943772 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p658iLaW024132 for ; Tue, 5 Jul 2011 08:44:21 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754353Ab1GEIoF (ORCPT ); Tue, 5 Jul 2011 04:44:05 -0400 Received: from fgwmail5.fujitsu.co.jp ([192.51.44.35]:50582 "EHLO fgwmail5.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754327Ab1GEIoB (ORCPT ); Tue, 5 Jul 2011 04:44:01 -0400 Received: from m2.gw.fujitsu.co.jp (unknown [10.0.50.72]) by fgwmail5.fujitsu.co.jp (Postfix) with ESMTP id 63E993EE0C1 for ; Tue, 5 Jul 2011 17:43:59 +0900 (JST) Received: from smail (m2 [127.0.0.1]) by outgoing.m2.gw.fujitsu.co.jp (Postfix) with ESMTP id 4C27C45DD6E for ; Tue, 5 Jul 2011 17:43:59 +0900 (JST) Received: from s2.gw.fujitsu.co.jp (s2.gw.fujitsu.co.jp [10.0.50.92]) by m2.gw.fujitsu.co.jp (Postfix) with ESMTP id 2B3CA45DD34 for ; Tue, 5 Jul 2011 17:43:59 +0900 (JST) Received: from s2.gw.fujitsu.co.jp (localhost.localdomain [127.0.0.1]) by s2.gw.fujitsu.co.jp (Postfix) with ESMTP id 1CDF61DB8038 for ; Tue, 5 Jul 2011 17:43:59 +0900 (JST) Received: from ml13.s.css.fujitsu.com (ml13.s.css.fujitsu.com [10.240.81.133]) by s2.gw.fujitsu.co.jp (Postfix) with ESMTP id CFC1A1DB802C for ; Tue, 5 Jul 2011 17:43:58 +0900 (JST) Received: from ml13.css.fujitsu.com (ml13 [127.0.0.1]) by ml13.s.css.fujitsu.com (Postfix) with ESMTP id A3C0EFD0034; Tue, 5 Jul 2011 17:43:58 +0900 (JST) Received: from [127.0.0.1] (unknown [10.124.101.143]) by ml13.s.css.fujitsu.com (Postfix) with ESMTP id 31323FD002E; Tue, 5 Jul 2011 17:43:58 +0900 (JST) X-SecurityPolicyCheck-FJ: OK by FujitsuOutboundMailChecker v1.3.1 Received: from KANE-LIFEBOOK[10.124.101.143] by KANE-LIFEBOOK (FujitsuOutboundMailChecker v1.3.1/9992[10.124.101.143]); Tue, 05 Jul 2011 17:43:46 +0900 (JST) Message-ID: <4E12CEBE.2090608@jp.fujitsu.com> Date: Tue, 05 Jul 2011 17:43:42 +0900 From: Kenji Kaneshige User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.9.2.18) Gecko/20110616 Thunderbird/3.1.11 MIME-Version: 1.0 To: Naoki Yanagimoto CC: linux-pci@vger.kernel.org, jbarnes@virtuousgeek.org Subject: Re: [patch] PCI: pciehp: Wait for 1 second in board_added() for a Valid Configuration Request References: <4E12C4F6.6060301@np.css.fujitsu.com> In-Reply-To: <4E12C4F6.6060301@np.css.fujitsu.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Tue, 05 Jul 2011 08:44:22 +0000 (UTC) (2011/07/05 17:01), Naoki Yanagimoto wrote: > From: Naoki Yanagimoto > > 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 Tested-by: Naoki Yanagimoto --- drivers/pci/hotplug/pciehp_ctrl.c | 3 +++ drivers/pci/hotplug/pciehp_hpc.c | 11 ++--------- 2 files changed, 5 insertions(+), 9 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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);