lkml.org 
[lkml]   [2017]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH v8 1/7] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
    On Thu, Oct 26, 2017 at 09:28:34PM +0800, Jeffy Chen wrote:
    > Add optional interrupts for PCIe WAKE# pin and PCI interrupt pin.
    >
    > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
    > ---
    >
    > 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 | 3 +++
    > 1 file changed, 3 insertions(+)
    >
    > diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt
    > index c77981c5dd18..faed405811cd 100644
    > --- a/Documentation/devicetree/bindings/pci/pci.txt
    > +++ b/Documentation/devicetree/bindings/pci/pci.txt
    > @@ -24,3 +24,6 @@ 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 and "pci"
    > + for PCI interrupt.

    Similar criticism to what Rob made on patch 4: this file already says "a
    host bridge driver implementation may support the following properties",
    so this property is clearly not for child devices. And so having the
    "PCI interrupt" here doesn't make much sense.

    Similarly, you're documenting "wakeup" here as a host bridge property,
    but then patch 7 is adding per-device support it seems? That seems
    wrong.

    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?

    And again, describing your intentions a little better in the commit
    message would make this clearer. Then we could tell which way you
    intended this to work...

    Brian

    \
     
     \ /
      Last update: 2017-10-27 04:34    [W:2.274 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site