From patchwork Sat Nov 23 00:51:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 3224641 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2A2049F461 for ; Sat, 23 Nov 2013 00:51:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 02F162079E for ; Sat, 23 Nov 2013 00:51:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EA30420635 for ; Sat, 23 Nov 2013 00:51:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755794Ab3KWAvI (ORCPT ); Fri, 22 Nov 2013 19:51:08 -0500 Received: from mail-ie0-f171.google.com ([209.85.223.171]:65291 "EHLO mail-ie0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755761Ab3KWAvH (ORCPT ); Fri, 22 Nov 2013 19:51:07 -0500 Received: by mail-ie0-f171.google.com with SMTP id ar20so3456974iec.30 for ; Fri, 22 Nov 2013 16:51:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=d81OafS7I7H8kBlRM+L4ukl3x9awkrLhVQZtwGSs4Rs=; b=cUDV63PGFFTe8tGs3q8tzyQbTdqiyT+/FVTOnlar6eAmTcW8zTdFsV0D3Fc6Nev0lV HMoyt/T6405lKwOng8KJjYrsfp4yDatyd/w6cVsuKiIucRRRhXME2wm4qDF1DBoDLwLS Qg5Jala+udSP9Q35LeGGbyfZotmWI4J+42ArrFH8C0unrp3ZBa595EhmsM/wcBv83rBP IERlMzQFXUxNewAAiwwVsTBah35SBJJSQkrvM6iwwSxjJKNIro2OZ7Pq/wWbapTCVO3N iWhxrkRKPQzSxq+kFHh0yiV9Q6AmGPmPMaAEH9yUM49XKm0ElXj2LQlE2dnLq4vH69hk djbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=d81OafS7I7H8kBlRM+L4ukl3x9awkrLhVQZtwGSs4Rs=; b=WsWU45IfLBPzzlOQw7nlqmPm5tLMh5+enC93G50KBPJmoHFwuZ+K05rVjletXLIVHI QDlTksHjtsfeTxvFSAAkbX6jA8rptk0sVC4PGh48xh1B3NyJ/FMO/TdLqIA4jXSLYC58 ZZE6Duj7bZHUGmF2YziCKG7IZZ+taCBAnvVm4DN+kQC6eXiP7jbZqMzsQ7M+wVIs2d+r g70bNGW34SupYFeBmZUCwInD5glBZpQl9bzl+bAKH/rc+rCLpki31RCTAf89MwmZ1vrN rBG6t7AX3cgPfXg58LsyFsGaQhkZLXRitEyrB+sGOZk3tj7EldTjYlFPiKiaXOt/Y8ad Eusw== X-Gm-Message-State: ALoCoQlIXqWHjZlK6jSzDRpSLRMHiYaEaDeDAZAGsPWram64qZxmArxoviJbqgRygHcIAo+6mKn46f+fvKSZzOTMXoVbSPS/ZT8fa46XCgKFNT2+41wdkdepviH6ikHPXuPxxEuzt+aUjhzbnBuVHsK+N00Fyq8pdJNUg42jr8/nWUQIVsrNJwpMsxeD4iYssAmhGO0qyFt8NruN0N9xbHDnu0lKTe13Cg== X-Received: by 10.50.30.229 with SMTP id v5mr4635810igh.27.1385167865521; Fri, 22 Nov 2013 16:51:05 -0800 (PST) Received: from google.com ([172.16.48.54]) by mx.google.com with ESMTPSA id q6sm11811375igi.0.2013.11.22.16.51.04 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 22 Nov 2013 16:51:05 -0800 (PST) Date: Fri, 22 Nov 2013 17:51:02 -0700 From: Bjorn Helgaas To: Rajat Jain Cc: "linux-pci@vger.kernel.org" , linux hotplug mailing , Kenji Kaneshige , Yijing Wang , Greg KH , Tom Nguyen , Kristen Accardi , Rajat Jain , Rajat Jain , Guenter Roeck Subject: Re: [PATCH] pciehp: Acknowledge the spurious "cmd completed" event. Message-ID: <20131123005102.GA14690@google.com> References: <52797238.8070304@gmail.com> <20131106002505.GC3359@google.com> <20131108012352.GE2955@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FSL_HELO_FAKE, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Nov 11, 2013 at 01:26:06PM -0800, Rajat Jain wrote: > Hello, > > >> With my patch, we'd eliminate the 1 second delay and the second line > >> of output above. If we also want to get rid of the first line > >> "Unexpected CMD_COMPLETED..." also, that would probably have to be > >> solved by changing the behavior to assume that CC shall be generated > >> for every SLOTCTRL register write. > > > > I *do* want to get rid of the "Unexpected CMD_COMPLETED" complaint. > > We shouldn't complain about hardware that is working perfectly fine. > > I don't know the best way to do that yet. I have found a box with the > > same hardware that was fixed by 5808639bfa98, so I hope to play with > > it and figure out something that will work nicely for both scenarios. > > Please keep posted :-) > > > > > BTW, can you open a report at bugzilla.kernel.org and attach your > > "lspci -vvxxx" and dmesg output? When we eventually merge a fix, I'd > > like to have the details archived somewhere. > > Done: > > https://bugzilla.kernel.org/show_bug.cgi?id=64821 > > Please let me know if you need any help in trying something out - I'd > be more than keen to help - write code or test. What do you think of the patch below? I'm afraid we'll trip over a few other old parts similar to the 82801H, but I'd rather do that than penalize the parts that work correctly. PCI: pciehp: Support command completion even with no hotplug hardware From: Bjorn Helgaas Commit 5808639bfa98 ("pciehp: fix slow probing") fixed a slow probing problem on hardware that doesn't conform to the spec, but caused a similar problem for hardware that *does* conform to the spec. Per PCIe 3.0, sec 7.8.10 and 6.7.3.2, any write to Slot Control generates a hot-plug command. Ports that can accept new commands with no delay can set the "No Command Completed Support" bit. Otherwise the port must indicate its completion of the command and readiness to accept a new command with a "command completed event." 5808639bfa98 assumes ports that lack a power controller, power indicator, attention indicator, and interlock will not generate completion events, even if they neglect to set "No Command Completed Support." But on ports that lack those elements and *do* support command completion notification, it causes: Unexpected CMD_COMPLETED. Need to wait for command completed event. Command not completed in 1000 msec and forces us to wait for a 1 second timeout. This patch makes the 5808639bfa98 workaround into a quirk that's applied only to devices known to be broken, currently just Intel 82801H ports. There are probably other similarly-broken devices that may now probe slowly, but I don't know how to catch them all without penalizing the ones that play by the rules. Link: https://bugzilla.kernel.org/show_bug.cgi?id=64821 Reported-by: Rajat Jain Signed-off-by: Bjorn Helgaas Tested-by: Guenter Roeck --- drivers/pci/hotplug/pciehp_hpc.c | 39 +++++++++++++++++++++++++++++--------- 1 file changed, 30 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 diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 3eea3fdd4b0b..2fd2bd59e07f 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -881,6 +881,34 @@ static inline void dbg_ctrl(struct controller *ctrl) ctrl_info(ctrl, "Slot Control : 0x%04x\n", reg16); } +static int pciehp_no_command_complete(struct controller *ctrl) +{ + struct pcie_device *dev = ctrl->pcie; + struct pci_dev *pdev = dev->port; + u16 vendor, device; + + if (NO_CMD_CMPL(ctrl)) + return 1; + + /* + * Controller should notify on command completion unless the "No + * Command Completed Support" bit is set. But some hardware does + * not. See https://bugzilla.kernel.org/show_bug.cgi?id=10751 + */ + if (!(POWER_CTRL(ctrl) | ATTN_LED(ctrl) | PWR_LED(ctrl) | EMI(ctrl))) { + vendor = pdev->vendor; + device = pdev->device; + if (vendor == PCI_VENDOR_ID_INTEL && + device >= 0x283f && device <= 0x2849) { + dev_info(&dev->device, "device [%04x:%04x] does not notify on hotplug command completion\n", + vendor, device); + return 1; + } + } + + return 0; +} + struct controller *pcie_init(struct pcie_device *dev) { struct controller *ctrl; @@ -902,15 +930,8 @@ struct controller *pcie_init(struct pcie_device *dev) mutex_init(&ctrl->ctrl_lock); init_waitqueue_head(&ctrl->queue); dbg_ctrl(ctrl); - /* - * Controller doesn't notify of command completion if the "No - * Command Completed Support" bit is set in Slot Capability - * register or the controller supports none of power - * controller, attention led, power led and EMI. - */ - if (NO_CMD_CMPL(ctrl) || - !(POWER_CTRL(ctrl) | ATTN_LED(ctrl) | PWR_LED(ctrl) | EMI(ctrl))) - ctrl->no_cmd_complete = 1; + + ctrl->no_cmd_complete = pciehp_no_command_complete(ctrl); /* Check if Data Link Layer Link Active Reporting is implemented */ if (pciehp_readl(ctrl, PCI_EXP_LNKCAP, &link_cap)) {