linux-kernel - Re: [RFC PATCH v10 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: <20171101210541.vacfufnn2ms65xrw@rob-hp-laptop>
Date:   Wed, 1 Nov 2017 16:05:41 -0500
From:   Rob Herring <robh@...nel.org>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Jeffy Chen <jeffy.chen@...k-chips.com>,
        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,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [RFC PATCH v10 1/7] dt-bindings: PCI: Add definition of PCIe
 WAKE# irq and PCI irq

On Fri, Oct 27, 2017 at 01:45:17PM -0700, Brian Norris wrote:
> Hi,
> 
> On Fri, Oct 27, 2017 at 03:26:06PM +0800, Jeffy Chen wrote:
> > We are going to handle PCIe WAKE# pin for PCI bus bridges and PCI
> > devices in the pci core, so add definitions of the optional PCIe
> > WAKE# pin for PCI bus bridges and PCI devices.
> > 
> > Also add an definition of the optional PCI interrupt pin for PCI
> > devices to distinguish it from the PCIe WAKE# pin.
> > 
> > Signed-off-by: Jeffy Chen <jeffy.chen@...k-chips.com>
> > ---
> > 
> > Changes in v10: None
> > Changes in v9:
> > Add section for PCI devices and rewrite the commit message.
> > 
> > Changes in v8:
> > Add optional "pci", and rewrite commit message.
> > 
> > Changes in v7: None
> > Changes in v6: None
> > Changes in v5:
> > Move to pci.txt
> > 
> > Changes in v3: None
> > Changes in v2: None
> > 
> >  Documentation/devicetree/bindings/pci/pci.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
> > index c77981c5dd18..d4406d4e15ad 100644
> > --- a/Documentation/devicetree/bindings/pci/pci.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci.txt
> > @@ -24,3 +24,11 @@ driver implementation may support the following properties:
> >     unsupported link speed, for instance, trying to do training for
> >     unsupported link speed, etc.  Must be '4' for gen4, '3' for gen3, '2'
> >     for gen2, and '1' for gen1. Any other values are invalid.
> > +- interrupts: Interrupt specifier for each name in interrupt-names.
> > +- interrupt-names: May contains "wakeup" for PCIe WAKE# interrupt.
> 
> s/contains/contain/
> 
> > +
> > +PCI devices have standardized Device Tree bindings:
> 
> This line is a little unclear, especially since there *is* an old
> documented standard, yet the following text is actually introducing new,
> non-standard additions.
> 
> > +
> > +- interrupts: Interrupt specifier for each name in interrupt-names.
> > +- interrupt-names: May contains "wakeup" for PCIe WAKE# interrupt and "pci" for
> 
> s/contains/contain/
> 
> > +  PCI interrupt.
> 
> IMO, since you're trying to augment a standardized binding, you need to
> be a lot clearer here. I expect you should mention the existing standard
> (that devices may optionally include an 'interrupts' property that
> represents the legacy PCI interrupt) and how you're augmenting it (that
> additional interrupts can be supported optionally, but they require a
> corresponding 'interrupt-names' property).

There's an additional complication that I'd guess the wakeup is 
typically a GPIO line and hence a different parent. We have 2 options 
there. The first is interrupts-extended which is generally implicitly 
supported (i.e. we only document interrupts). The second is we already 
have interrupt-map if we have legacy interrupts and can map to different 
parents. For this to work, we'd have to use a number >4 for the wakeup 
interrupts.

Rob