From patchwork Thu Jun 15 07:02:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kai-Heng Feng X-Patchwork-Id: 9788159 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8E2FD60348 for ; Thu, 15 Jun 2017 07:03:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7F6FD284F8 for ; Thu, 15 Jun 2017 07:03:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 73FF028520; Thu, 15 Jun 2017 07:03:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0AB18284F8 for ; Thu, 15 Jun 2017 07:03:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751948AbdFOHCl (ORCPT ); Thu, 15 Jun 2017 03:02:41 -0400 Received: from mail-wr0-f176.google.com ([209.85.128.176]:35291 "EHLO mail-wr0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750814AbdFOHC2 (ORCPT ); Thu, 15 Jun 2017 03:02:28 -0400 Received: by mail-wr0-f176.google.com with SMTP id q97so7954952wrb.2 for ; Thu, 15 Jun 2017 00:02:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=1Uycjs1dj8pWlVksEQh5yG9427FNf8beA1BZ3uA8558=; b=RZlU3T5YFNClJcWwfuMJ4X5v08eT0lor6X6zaNClIE52DGVlPs1l8OFRuo9Bzt4spt jtORhIpdqT7CRkEK24kQPSAMq7JHao6SY/Aqi9LgM6kqW/AkryIX63kJf2M4/TuYveBh eKEHCaf8gkFwDIug8eX96DrblXsmEL7LiUl98DML5kM18lEzQLy9Vw/bDHxjoix5MmVn zrES/F1tLXk6n5wqOJXgJjnI/ZUomqHVyvBGaIZQ5KETgEj/VEgpBgyXvVbi/jVfZbyM rKNuQyw1wWo4EcAjXv/D/TkujKXZv1OBUFXB7IDfVuRajpxQX+Bv4RmGIj5BN6ki25gm FSeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=1Uycjs1dj8pWlVksEQh5yG9427FNf8beA1BZ3uA8558=; b=VaIRRUz77vwo5cGrN7UKXWBaDzAsHWOQ+HCcTlntazcbMm9vRoc4aSe8D6vLZd4n9N l7CXYoiqmSy83jqLC8kBe+mJ6TFSKpd8j3SLVK8y5dl6S2cDOkl8xrprPSZuLFhZ1CMj GfMoGJ1xMT/hwqor1eh07ZWAbxhManP6oYUT5ND6GDufodXb3XDCFIyO+KYSTatPWukl yQ2n5IoYUGsxy4hnVi+4K6MGeqU+TcY7y1KfNg/NaqB6e0imGwhPIaOInjK6W02QVUye GH2gUORQbB8JsLYMsTW5be5r8j4Z/HrGZ78TbN4vSmeKQF81StVgCkK3yaHwqEaYh67w mQsA== X-Gm-Message-State: AKS2vOwh8HA1FODrK6erbx5xiyYfBxB5uMtLFVBPU3US0ZgCbU1hsw8h WPe1kILPTJqJ8l7z3qE1NFKfRgHUXuCd X-Received: by 10.28.136.196 with SMTP id k187mr2371969wmd.103.1497510146687; Thu, 15 Jun 2017 00:02:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.189.133 with HTTP; Thu, 15 Jun 2017 00:02:25 -0700 (PDT) In-Reply-To: References: <20170613172849.GA7012@bhelgaas-glaptop.roam.corp.google.com> From: Kai-Heng Feng Date: Thu, 15 Jun 2017 15:02:25 +0800 Message-ID: Subject: Re: [PATCH] usb: host: ehci: workaround PME bug on AMD EHCI controller To: Alan Stern Cc: Bjorn Helgaas , Bjorn Helgaas , gregkh@linuxfoundation.org, USB list , linux-pci@vger.kernel.org, LKML , "Rafael J. Wysocki" , linux-pm@vger.kernel.org Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jun 15, 2017 at 2:55 AM, Alan Stern wrote: > On Tue, 13 Jun 2017, Bjorn Helgaas wrote: > >> [+cc Rafael, linux-pm] >> >> On Tue, Jun 13, 2017 at 12:21:15PM +0800, Kai-Heng Feng wrote: >> > On Mon, Jun 12, 2017 at 10:18 PM, Alan Stern wrote: >> > > Let's get some help from people who understand PCI well. >> > > >> > > Here's the general problem: Kai-Heng has a PCI-based USB host >> > > controller that advertises wakeup capability from D3, but it doesn't >> > > assert PME# from D3 when it should. For "lspci -vv" output, see >> > > >> > > http://marc.info/?l=linux-usb&m=149570231732519&w=2 >> > > >> > > On Mon, 12 Jun 2017, Kai-Heng Feng wrote: >> > > >> > >> On Mon, Jun 12, 2017 at 3:04 PM, Kai-Heng Feng >> > >> wrote: >> > >> > On Fri, Jun 9, 2017 at 10:43 PM, Alan Stern wrote: >> > >> >> On Fri, 9 Jun 2017, Kai-Heng Feng wrote: >> > >> >> >> > >> >> Is this really the right solution? Maybe it would be better to allow >> > >> >> the controller to go into D3 provided no wakeup signal is needed. You >> > >> >> could do: >> > >> >> >> > >> >> device_set_wakeup_capable(&pdev->dev, 0); >> > >> > >> > >> > This doesn't work. >> > >> > After applying this function, still nothing happens when devices get plugged in. >> > >> > IIUC this function disable the wakeup function, but what I want to do >> > >> > here is to have PME signal works even when runtime PM is enabled. >> > > >> > > This may indicate a bug in either the PCI or USB stacks (or both!). If >> > > a driver requires wakeup capability from runtime suspend but the device >> > > does not provide it, the PCI core should not allow the device to go >> > > into runtime suspend. Or is that the driver's responsibility? >> > > >> > >> > I also saw some legacy PCI PM stuff, so I also tried: >> > >> > device_set_wakeup_capable(&pdev->dev, 1); >> > >> > ...doesn't work either. >> > >> > >> > >> >> >> > >> >> Another alternative is to put the controller into D2 instead of D3, but >> > >> >> (1) I don't know how to do that, and (2) we don't know if wakeup >> > >> >> signalling works any better in D2 than it does in D3. >> > >> > >> > >> > I'll try if D2 works. >> > >> >> > >> Put the device into D2 instead of D3 can make the wakeup signaling >> > >> work, i.e. USB devices can be correctly detected after plugged into >> > >> EHCI port. >> > >> >> > >> Do you think this alternative an acceptable workaround? >> > > >> > > Yes, it is. The difficulty is that I don't know how to tell the PCI >> > > core that the device should go in D2 during runtime suspend instead of >> > > D3. Some sort of quirk may be needed -- perhaps Bjorn can help. > >> The lspci output [1] shows: >> >> 00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI Controller (rev 39) (prog-if 20 [EHCI]) >> Capabilities: [c0] Power Management version 2 >> Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+) >> Status: D3 NoSoftRst- PME-Enable+ DSel=0 DScale=0 PME- >> Bridge: PM- B3+ >> >> The device claims it can assert PME# from D3hot. If it can't, that >> sounds like a hardware defect that should be addressed with a quirk. >> Ideally we would also have a pointer to the AMD hardware erratum. >> >> Is the following path involved here? >> >> pci_finish_runtime_suspend >> target_state = pci_target_state() >> if (device_may_wakup()) >> if (dev->pme_support) >> ... >> pci_set_power_state(..., target_state) >> >> If so, I would naively expect that a quirk could clear the >> PCI_PM_CAP_PME_D3 and PCI_PM_CAP_PME_D3cold bits in dev->pme_support, >> and pci_target_state() would then avoid selecting D3 or D3cold. But >> I'm not an expert in power management. > > That's a good idea. However, we should apply the quirk only when it is > needed. Which means we need to know the numeric values for the PCI > IDs. Also, this will help searching for published errata. > > Kai-Heng, what does "lspci -nvs 00:12.0" show? 00:12.0 0c03: 1022:7808 (rev 39) (prog-if 20 [EHCI]) Subsystem: 1028:0732 Flags: bus master, 66MHz, medium devsel, latency 32, IRQ 18 Memory at fe769000 (32-bit, non-prefetchable) [size=256] Capabilities: [c0] Power Management version 2 Capabilities: [e4] Debug port: BAR=1 offset=00e0 Kernel driver in use: ehci-pci Here's the diff that can make it work: dev_printk(KERN_DEBUG, &dev->dev, "PME# supported from%s%s%s%s%s\n", If you think this is OK, I'll resend the patch. diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 1a14ca8965e6..7bd278535ab3 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2208,6 +2208,12 @@ void pci_pm_init(struct pci_dev *dev) } pmc &= PCI_PM_CAP_PME_MASK; + + if (unlikely(dev->vendor == 0x1022 && dev->device == 0x7808)) { + dev_info(&dev->dev, "PME# does not work under D3, disabling it\n"); + pmc &= ~(PCI_PM_CAP_PME_D3 | PCI_PM_CAP_PME_D3cold); + } + if (pmc) {