From patchwork Tue Jun 13 04:21:15 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: 9783041 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 58C86602C9 for ; Tue, 13 Jun 2017 04:21:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4B73C2854A for ; Tue, 13 Jun 2017 04:21:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3FE6A28591; Tue, 13 Jun 2017 04:21:26 +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 CB6122854A for ; Tue, 13 Jun 2017 04:21:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751906AbdFMEVT (ORCPT ); Tue, 13 Jun 2017 00:21:19 -0400 Received: from mail-wr0-f171.google.com ([209.85.128.171]:36516 "EHLO mail-wr0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751864AbdFMEVR (ORCPT ); Tue, 13 Jun 2017 00:21:17 -0400 Received: by mail-wr0-f171.google.com with SMTP id 36so1469291wry.3 for ; Mon, 12 Jun 2017 21:21:16 -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=q3Ve8/Crq4BaLkZ1D1EfFhSjzFC5/jTeIcGxYsGPkaY=; b=SEfBD5aQtQjPIfcIpxTlCs5Ts9GgP+0YsYAQv6S7T+Xen96Qo23ABV6kUwCNttUuK2 /2B7UnR+NW6Ebd8m/WigwQZMphStRdMx9k6m0W1xhIqFTacvTxpX2bZ+heCRQWOk62gA NDIFkurLcPlIyejTF/y2a9/bbWVo2exOn6YU23sR/6GwxDtBw/IEY8F8ScSrS7KMzmqT ib7Rxz49UmThf9Bl+HUI4KxVqtRVGdWZFh94PXA/9PEnTKacp3RGSODwq0melc0LHJl5 LDZ6MBUdfqvLvgLORiDm/e2rP5NjxInojQPjIQB6TrTk0mcrFrxrXrAbbbDMhVgyzq7x u6cQ== 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=q3Ve8/Crq4BaLkZ1D1EfFhSjzFC5/jTeIcGxYsGPkaY=; b=TIOWweEdV0yaVmrGRq1AwHKr4WI6BzkH6NvTuaGyYTxvU1lvS/qOneUQxuvI1lffYi nz4fOgkSLPM94nMsXGbUg9MR4zGPBDLm4REpa9d3OG01qKBVzoEpNA3w6BQbMck24+9L wRTZxkAIfC5B8l89gLkQFpJhA9oe7+NAVIR8D7q8uKjaMmmOff1f4uSj1REQ08sJ29xl al/C5SfHzn34EC0gQBSI96+eW8h1u0VFgjH42tXYWHmM3GeXHJIICghYw7rd80yNt6LU KdtkPDajn9xWbVWRNlcB5jPXOJxPyjHFCaozHjQCTtc0P9SsjWQAW0qeiBmJCV5tP3pB 0itQ== X-Gm-Message-State: AKS2vOx6HYBddSre6vamEFrXBwTQL4fsjXFYjmaIyqIbkZP4W2Ra1uIX PMsvhwLRItPG07mCq8FeUE7JNDaS86Qg X-Received: by 10.223.154.15 with SMTP id z15mr1344297wrb.136.1497327675831; Mon, 12 Jun 2017 21:21:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.189.133 with HTTP; Mon, 12 Jun 2017 21:21:15 -0700 (PDT) In-Reply-To: References: From: Kai-Heng Feng Date: Tue, 13 Jun 2017 12:21:15 +0800 Message-ID: Subject: Re: [PATCH] usb: host: ehci: workaround PME bug on AMD EHCI controller To: Alan Stern Cc: Bjorn Helgaas , gregkh@linuxfoundation.org, USB list , linux-pci@vger.kernel.org, LKML 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 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. > FWIW, this is the diff I used to make the controller transits to D2 instead of D3: diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 563901cd9c06..36663688404a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -922,6 +922,9 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) if (dev->current_state == state) return 0; + if (state > PCI_D2 && (dev->dev_flags & PCI_DEV_FLAGS_MAX_D2)) + state = PCI_D2; + __pci_start_power_transition(dev, state); /* This device is quirked not to be put into D3, so diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c index 93326974ff4b..a2c1fe62974a 100644 --- a/drivers/usb/host/ehci-pci.c +++ b/drivers/usb/host/ehci-pci.c @@ -181,6 +181,8 @@ static int ehci_pci_setup(struct usb_hcd *hcd) if (pdev->device == 0x7808) { ehci->use_dummy_qh = 1; ehci_info(ehci, "applying AMD SB700/SB800/Hudson-2/3 EHCI dummy qh workaround\n"); + + pdev->dev_flags |= PCI_DEV_FLAGS_MAX_D2; } break; case PCI_VENDOR_ID_VIA: diff --git a/include/linux/pci.h b/include/linux/pci.h index 8039f9f0ca05..6f86f2aa8548 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -188,6 +188,7 @@ enum pci_dev_flags { * the direct_complete optimization. */ PCI_DEV_FLAGS_NEEDS_RESUME = (__force pci_dev_flags_t) (1 << 11), + PCI_DEV_FLAGS_MAX_D2 = (__force pci_dev_flags_t) (1 << 12), }; enum pci_irq_reroute_variant {