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