linux-kernel - Re: [RFC PATCH v8 1/7] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171027054025.GA85582@google.com>
Date:   Thu, 26 Oct 2017 22:40:28 -0700
From:   Brian Norris <briannorris@...omium.org>
To:     jeffy <jeffy.chen@...k-chips.com>
Cc:     linux-kernel@...r.kernel.org, bhelgaas@...gle.com,
        linux-pm@...r.kernel.org, tony@...mide.com,
        shawn.lin@...k-chips.com, rjw@...ysocki.net, dianders@...omium.org,
        devicetree@...r.kernel.org, linux-pci@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [RFC PATCH v8 1/7] dt-bindings: PCI: Add definition of PCIe
 WAKE# irq and PCI irq

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