| Message ID | 20171226020806.32710-2-jeffy.chen@rock-chips.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PCI: rockchip: Move PCIe WAKE# handling into pci core | expand |
* Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]: > We are going to handle PCIe WAKE# pin for PCI devices in the pci core, > so add definitions of the optional PCIe WAKE# pin for PCI devices. > > Also add an definition of the optional PCI interrupt pin for PCI > devices to distinguish it from the PCIe WAKE# pin. > --- a/Documentation/devicetree/bindings/pci/pci.txt > +++ b/Documentation/devicetree/bindings/pci/pci.txt > @@ -24,3 +24,13 @@ 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. > + > +PCI devices may support the following properties: This should say PCI ports instead of PCI devices. > +- interrupts: Interrupt specifier for each name in interrupt-names. > +- interrupt-names: > + May contain "wakeup" for PCIe WAKE# interrupt and "pci" for PCI interrupt. > + The PCI devices may optionally include an 'interrupts' property that > + represents the legacy PCI interrupt. And when we try to specify the PCIe > + WAKE# pin, a corresponding 'interrupt-names' property is required to > + distinguish them. > -- > 2.11.0 > >
On Fri, Dec 29, 2017 at 6:57 PM, Tony Lindgren <tony@atomide.com> wrote: > * Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]: >> We are going to handle PCIe WAKE# pin for PCI devices in the pci core, >> so add definitions of the optional PCIe WAKE# pin for PCI devices. >> >> Also add an definition of the optional PCI interrupt pin for PCI >> devices to distinguish it from the PCIe WAKE# pin. > >> --- a/Documentation/devicetree/bindings/pci/pci.txt >> +++ b/Documentation/devicetree/bindings/pci/pci.txt >> @@ -24,3 +24,13 @@ 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. >> + >> +PCI devices may support the following properties: > > This should say PCI ports instead of PCI devices. No, it is more accurate to say "PCI devices". Well, it actually gets somewhat confusing, because in the PCI terminology a "PCI device" means a physical piece of hardware that can be put into a single "slot" (think socket on a board) and may consist up to 8 functional units called "functions" which are each represented by a struct pci_dev. So there may be up to 8 struct pci_dev objects per "PCI device" (as per the standard language) and, BTW, drivers bind to functions (via the struct pci_dev objects). Now, WAKE# is shared by all functions within the same "PCI device" (I'm not sure if the standard specifies that directly, but at least it appears to be treated as an obvious physical limitation), so it may be useful to represent the "slot" or "device" level in the DT even though it has no struct device based representation in the kernel. >> +- interrupts: Interrupt specifier for each name in interrupt-names. >> +- interrupt-names: >> + May contain "wakeup" for PCIe WAKE# interrupt and "pci" for PCI interrupt. >> + The PCI devices may optionally include an 'interrupts' property that >> + represents the legacy PCI interrupt. And when we try to specify the PCIe >> + WAKE# pin, a corresponding 'interrupt-names' property is required to >> + distinguish them. >> -- >> 2.11.0 >> >>
On Sat, Dec 30, 2017 at 12:50 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Fri, Dec 29, 2017 at 6:57 PM, Tony Lindgren <tony@atomide.com> wrote: >> * Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]: >>> We are going to handle PCIe WAKE# pin for PCI devices in the pci core, >>> so add definitions of the optional PCIe WAKE# pin for PCI devices. >>> >>> Also add an definition of the optional PCI interrupt pin for PCI >>> devices to distinguish it from the PCIe WAKE# pin. >> >>> --- a/Documentation/devicetree/bindings/pci/pci.txt >>> +++ b/Documentation/devicetree/bindings/pci/pci.txt >>> @@ -24,3 +24,13 @@ 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. >>> + >>> +PCI devices may support the following properties: >> >> This should say PCI ports instead of PCI devices. > > No, it is more accurate to say "PCI devices". > > Well, it actually gets somewhat confusing, because in the PCI > terminology a "PCI device" means a physical piece of hardware that can > be put into a single "slot" (think socket on a board) and may consist > up to 8 functional units called "functions" which are each represented > by a struct pci_dev. So there may be up to 8 struct pci_dev objects > per "PCI device" (as per the standard language) and, BTW, drivers bind > to functions (via the struct pci_dev objects). > > Now, WAKE# is shared by all functions within the same "PCI device" > (I'm not sure if the standard specifies that directly, but at least it > appears to be treated as an obvious physical limitation), so it may be > useful to represent the "slot" or "device" level in the DT even though > it has no struct device based representation in the kernel. Within the convention that bridges represent "everything below them" as far as WAKE# is concerned, it can say "The following properties may be provided for PCI bridges:" and the description below should explain the convention. Thanks, Rafael
* Rafael J. Wysocki <rafael@kernel.org> [171229 23:52]: > On Fri, Dec 29, 2017 at 6:57 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]: > >> We are going to handle PCIe WAKE# pin for PCI devices in the pci core, > >> so add definitions of the optional PCIe WAKE# pin for PCI devices. > >> > >> Also add an definition of the optional PCI interrupt pin for PCI > >> devices to distinguish it from the PCIe WAKE# pin. > > > >> --- a/Documentation/devicetree/bindings/pci/pci.txt > >> +++ b/Documentation/devicetree/bindings/pci/pci.txt > >> @@ -24,3 +24,13 @@ 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. > >> + > >> +PCI devices may support the following properties: > > > > This should say PCI ports instead of PCI devices. > > No, it is more accurate to say "PCI devices". > > Well, it actually gets somewhat confusing, because in the PCI > terminology a "PCI device" means a physical piece of hardware that can > be put into a single "slot" (think socket on a board) and may consist > up to 8 functional units called "functions" which are each represented > by a struct pci_dev. So there may be up to 8 struct pci_dev objects > per "PCI device" (as per the standard language) and, BTW, drivers bind > to functions (via the struct pci_dev objects). OK then let's stick to "PCI devices". > Now, WAKE# is shared by all functions within the same "PCI device" > (I'm not sure if the standard specifies that directly, but at least it > appears to be treated as an obvious physical limitation), so it may be > useful to represent the "slot" or "device" level in the DT even though > it has no struct device based representation in the kernel. Yeah that makes sense as there is only one WAKE# line in the slot :) Regards, Tony
* Rafael J. Wysocki <rafael@kernel.org> [171230 00:34]: > On Sat, Dec 30, 2017 at 12:50 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Dec 29, 2017 at 6:57 PM, Tony Lindgren <tony@atomide.com> wrote: > >> * Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:11]: > >>> We are going to handle PCIe WAKE# pin for PCI devices in the pci core, > >>> so add definitions of the optional PCIe WAKE# pin for PCI devices. > >>> > >>> Also add an definition of the optional PCI interrupt pin for PCI > >>> devices to distinguish it from the PCIe WAKE# pin. > >> > >>> --- a/Documentation/devicetree/bindings/pci/pci.txt > >>> +++ b/Documentation/devicetree/bindings/pci/pci.txt > >>> @@ -24,3 +24,13 @@ 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. > >>> + > >>> +PCI devices may support the following properties: > >> > >> This should say PCI ports instead of PCI devices. > > > > No, it is more accurate to say "PCI devices". > > > > Well, it actually gets somewhat confusing, because in the PCI > > terminology a "PCI device" means a physical piece of hardware that can > > be put into a single "slot" (think socket on a board) and may consist > > up to 8 functional units called "functions" which are each represented > > by a struct pci_dev. So there may be up to 8 struct pci_dev objects > > per "PCI device" (as per the standard language) and, BTW, drivers bind > > to functions (via the struct pci_dev objects). > > > > Now, WAKE# is shared by all functions within the same "PCI device" > > (I'm not sure if the standard specifies that directly, but at least it > > appears to be treated as an obvious physical limitation), so it may be > > useful to represent the "slot" or "device" level in the DT even though > > it has no struct device based representation in the kernel. > > Within the convention that bridges represent "everything below them" > as far as WAKE# is concerned, it can say "The following properties may > be provided for PCI bridges:" and the description below should explain > the convention. Sounds good to me. Regards, Tony
diff --git a/Documentation/devicetree/bindings/pci/pci.txt b/Documentation/devicetree/bindings/pci/pci.txt index c77981c5dd18..3045ac452f27 100644 --- a/Documentation/devicetree/bindings/pci/pci.txt +++ b/Documentation/devicetree/bindings/pci/pci.txt @@ -24,3 +24,13 @@ 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. + +PCI devices may support the following properties: + +- interrupts: Interrupt specifier for each name in interrupt-names. +- interrupt-names: + May contain "wakeup" for PCIe WAKE# interrupt and "pci" for PCI interrupt. + The PCI devices may optionally include an 'interrupts' property that + represents the legacy PCI interrupt. And when we try to specify the PCIe + WAKE# pin, a corresponding 'interrupt-names' property is required to + distinguish them.
We are going to handle PCIe WAKE# pin for PCI devices in the pci core, so add definitions of the optional PCIe WAKE# pin for 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@rock-chips.com> --- Changes in v12: None Changes in v11: Only add irq definitions for PCI devices and rewrite the commit message. 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 | 10 ++++++++++ 1 file changed, 10 insertions(+)