From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Norris Subject: Re: [RFC PATCH v8 1/7] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq Date: Thu, 26 Oct 2017 22:40:28 -0700 Message-ID: <20171027054025.GA85582@google.com> References: <20171026132840.20946-1-jeffy.chen@rock-chips.com> <20171026132840.20946-2-jeffy.chen@rock-chips.com> <20171027023331.GA11665@google.com> <59F2A2BA.5030307@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-io0-f195.google.com ([209.85.223.195]:50531 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750790AbdJ0Fkc (ORCPT ); Fri, 27 Oct 2017 01:40:32 -0400 Received: by mail-io0-f195.google.com with SMTP id 97so10284771iok.7 for ; Thu, 26 Oct 2017 22:40:32 -0700 (PDT) Content-Disposition: inline In-Reply-To: <59F2A2BA.5030307@rock-chips.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: jeffy Cc: linux-kernel@vger.kernel.org, bhelgaas@google.com, linux-pm@vger.kernel.org, tony@atomide.com, shawn.lin@rock-chips.com, rjw@rjwysocki.net, dianders@chromium.org, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, Rob Herring , Mark Rutland Hi Jeffy, On Fri, Oct 27, 2017 at 11:06:34AM +0800, Jeffy Chen wrote: > On 10/27/2017 10:33 AM, Brian Norris wrote: > >In fact, I'm pretty sure this series fails to actually look in the host > >bridge for the "wakeup" interrupt at all! Did you actually test this? > actually it could... > > static void *of_pci_setup(struct device *dev) > { > ... > device_init_wakeup(dev, false); > > dev_info(dev, "Wakeup IRQ %d\n", irq); > return data; > } > > [ 1.546561] OF: PCI: MEM 0xfa000000..0xfbdfffff -> 0xfa000000 > [ 1.553154] OF: PCI: IO 0xfbe00000..0xfbefffff -> 0xfbe00000 > [ 1.560859] rockchip-pcie f8000000.pcie: Wakeup IRQ 64 > [ 1.566555] rockchip-pcie f8000000.pcie: PCI host bridge to bus Hmm, I think I missed the .setup_host_bridge() stuff. So you do handle both. I'll have to take a little closer look tomorrow. But you definitely at least need to improve the documentation as mentioned. Another odd thing about this series is that the interrupt doesn't actually show up in /proc/interrupts, /sys/kernel/debug/gpio, or similar, seemingly because the wakeirq is requested/released every time we suspend/resume. So it's really not that obvious that the interrupt is being configured properly. That's not really a functional problem, necessarily, but it doesn't quite seem ideal. Brian