Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751071AbdFTChE (ORCPT ); Mon, 19 Jun 2017 22:37:04 -0400 Received: from mail-wm0-f48.google.com ([74.125.82.48]:36238 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750905AbdFTChB (ORCPT ); Mon, 19 Jun 2017 22:37:01 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170619174533.GA20550@bhelgaas-glaptop.roam.corp.google.com> From: Kai-Heng Feng Date: Tue, 20 Jun 2017 10:36:59 +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 Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1476 Lines: 32 On Tue, Jun 20, 2017 at 2:32 AM, Alan Stern wrote: > > It's possible that the test was invalid. Kai-Heng did not say whether > /sys/.../power/wakeup was set to "enabled" for both the EHCI controller > and the USB root hub beneath it, before the test was started. If > either of them was set to "disabled" then we would not expect a plug or > unplug event to wake up the system. You are right, it's "disabled" on USB root hub. Changed it to "enabled", the test result remains the same. > > In any case, the controller should be set to the lowest power setting > that is consistent with the desired wakeup behavior. If wakeup is set > to "enabled" then the state should be D2 -- if possible. That's the > theory, anyway. If the system supports putting devices only into D3 > during S3 sleep then there's no choice, but if we do have a choice then > we should take it. > > BTW, I just noticed that pci_target_state() uses device_may_wakeup() to > get the desired wakeup behavior. That is correct for system sleep, but > it is wrong for runtime PM. For runtime PM, wakeup should be enabled > whenever the hardware allows it, so the test should be > device_can_wakeup(). > > This means that pci_target_state() should behave differently depending > on whether it is called from pci_prepare_to_sleep() or from > pci_finish_runtime_suspend(). Probably nobody noticed this before > because it usually doesn't make any difference. > > Alan Stern >