[RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core

[RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core

[Jeffy Chen]

Currently we are handling wake irq in mrvl wifi driver. Move it into
pci core.

Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).


Changes in v13:
Fix compiler error reported by kbuild test robot <fengguang.wu@intel.com>

Changes in v12:
Only add irq definitions for PCI devices and rewrite the commit message.
Enable the wake irq in noirq stage to avoid possible irq storm.

Changes in v11:
Address Brian's comments.
Only support 1-per-device PCIe WAKE# pin as suggested.
Move to pcie port as Brian suggested.

Changes in v10:
Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
since dedicated wakeirq will be lost in device_set_wakeup_enable(false).

Changes in v9:
Add section for PCI devices and rewrite the commit message.
Fix check error in .cleanup().
Move dedicated wakeirq setup to setup() callback and use
device_set_wakeup_enable() to enable/disable.
Rewrite the commit message.

Changes in v8:
Add optional "pci", and rewrite commit message.
Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
Rewrite the commit message.

Changes in v7:
Move PCIE_WAKE handling into pci core.

Changes in v6:
Fix device_init_wake error handling, and add some comments.

Changes in v5:
Move to pci.txt
Rebase.
Use "wakeup" instead of "wake"

Changes in v3:
Fix error handling.

Changes in v2:
Use dev_pm_set_dedicated_wake_irq.

Jeffy Chen (5):
  dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
  of/irq: Adjust of_pci_irq parsing for multiple interrupts
  mwifiex: Disable wakeup irq handling for pcie
  PCI / PM: Add support for the PCIe WAKE# signal for OF
  arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru

 Documentation/devicetree/bindings/pci/pci.txt | 10 ++++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 11 ++--
 drivers/net/wireless/marvell/mwifiex/main.c   |  4 ++
 drivers/of/of_pci_irq.c                       | 71 +++++++++++++++++++++++--
 drivers/pci/Makefile                          |  1 +
 drivers/pci/pci-driver.c                      | 10 ++++
 drivers/pci/pci-of.c                          | 75 +++++++++++++++++++++++++++
 include/linux/of_pci.h                        |  9 ++++
 8 files changed, 183 insertions(+), 8 deletions(-)
 create mode 100644 drivers/pci/pci-of.c

-- 
2.11.0

[RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Jeffy Chen]

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 v13: None
Changes in v12:
Only add irq definitions for PCI devices and rewrite the commit message.

Changes in v11: None
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(+)

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.
-- 
2.11.0

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Rob Herring]

On Tue, Dec 26, 2017 at 10:36:42AM +0800, Jeffy Chen wrote:
> 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.

By v13 you should drop "RFC". RFC implies not ready for merging.

> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v13: None
> Changes in v12:
> Only add irq definitions for PCI devices and rewrite the commit message.
> 
> Changes in v11: None
> 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(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Rafael J. Wysocki]

On Wednesday, December 27, 2017 12:35:52 AM CET Rob Herring wrote:
> On Tue, Dec 26, 2017 at 10:36:42AM +0800, Jeffy Chen wrote:
> > 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.
> 
> By v13 you should drop "RFC". RFC implies not ready for merging.

Which very much still is the case AFAICS.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[JeffyChen]

Hi Rafael and Rob,

Thanks for your reply.

On 12/27/2017 08:43 AM, Rafael J. Wysocki wrote:
> On Wednesday, December 27, 2017 12:35:52 AM CET Rob Herring wrote:
>> >On Tue, Dec 26, 2017 at 10:36:42AM +0800, Jeffy Chen wrote:
>>> > >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.
>> >
>> >By v13 you should drop "RFC". RFC implies not ready for merging.
> Which very much still is the case AFAICS.
>
maybe i should split this series, and send dt-binding patch and the pci 
irq parsing patch along without RFC ?

> Thanks,
> Rafael
>
>
>
>

[RFC PATCH v12 5/5] arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru

[Jeffy Chen]

Currently we are handling PCIe WAKE# irq in mrvl wifi driver.

Move it to rockchip pcie port since we are going to handle it in the
pci core.

Also avoid this irq been considered as the PCI interrupt pin in the
of_irq_parse_pci().

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v13: None
Changes in v12: None
Changes in v11:
Move to pcie port as Brian suggested.

Changes in v10: None
Changes in v9:
Rewrite the commit message.

Changes in v8:
Rewrite the commit message.

Changes in v7: None
Changes in v6: None
Changes in v5:
Use "wakeup" instead of "wake"

Changes in v3: None
Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 03f195025390..be41d363efd8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -719,15 +719,16 @@ ap_i2c_audio: &i2c8 {
 		#size-cells = <2>;
 		ranges;
 
+		interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pci", "wakeup";
+		pinctrl-names = "default";
+		pinctrl-0 = <&wlan_host_wake_l>;
+		wakeup-source;
+
 		mvl_wifi: wifi@0,0 {
 			compatible = "pci1b4b,2b42";
 			reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
 			       0x83010000 0x0 0x00100000 0x0 0x00100000>;
-			interrupt-parent = <&gpio0>;
-			interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&wlan_host_wake_l>;
-			wakeup-source;
 		};
 	};
 };
-- 
2.11.0

Re: [RFC PATCH v12 5/5] arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru

[Tony Lindgren]

* Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> [171226 02:41]:
> Currently we are handling PCIe WAKE# irq in mrvl wifi driver.
> 
> Move it to rockchip pcie port since we are going to handle it in the
> pci core.

Yes in the PCIe case, the pcie port node is the right place for
the wakeirq instead of the child the mvl_wifi node. So one
question further down below to verify this..

> Also avoid this irq been considered as the PCI interrupt pin in the
> of_irq_parse_pci().

The above paragraph needs a bit more clarification to be
readable :)

> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -719,15 +719,16 @@ ap_i2c_audio: &i2c8 {
>  		#size-cells = <2>;
>  		ranges;
>  
> +		interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "pci", "wakeup";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wlan_host_wake_l>;
> +		wakeup-source;
> +
>  		mvl_wifi: wifi@0,0 {
>  			compatible = "pci1b4b,2b42";
>  			reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>  			       0x83010000 0x0 0x00100000 0x0 0x00100000>;
> -			interrupt-parent = <&gpio0>;
> -			interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> -			pinctrl-names = "default";
> -			pinctrl-0 = <&wlan_host_wake_l>;
> -			wakeup-source;
>  		};
>  	};
>  };

So the above modifies pcie@0,0 node. And that node describes
the particular PCIe port that the WLAN is connected to instead
of describing the whole PCIe controller device, right?

If so, then yeah it's totally where the wakeirq should be
defined for a PCIe device in the dts file :)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 5/5] arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru

[Rafael J. Wysocki]

On Fri, Dec 29, 2017 at 6:55 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Jeffy Chen <jeffy.chen@rock-chips.com> [171226 02:41]:
>> Currently we are handling PCIe WAKE# irq in mrvl wifi driver.
>>
>> Move it to rockchip pcie port since we are going to handle it in the
>> pci core.
>
> Yes in the PCIe case, the pcie port node is the right place for
> the wakeirq instead of the child the mvl_wifi node. So one
> question further down below to verify this..

You seem to be using a convention by which the port represents the
whole "slot" or "PCI device" (as an entity consisting of up to 8
functions) connected to it.

That is fair enough as long as the port is not the top of a more
complex branch of the PCIe hierarchy, so maybe that case needs to be
made special somehow?

Also, I would document the convention by mentioning that the wakeup
signaled via that interrupt doesn't apply to the port itself, but to
the functions (endpoints) below it.

>> Also avoid this irq been considered as the PCI interrupt pin in the
>> of_irq_parse_pci().
>
> The above paragraph needs a bit more clarification to be
> readable :)
>
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
>> @@ -719,15 +719,16 @@ ap_i2c_audio: &i2c8 {
>>               #size-cells = <2>;
>>               ranges;
>>
>> +             interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
>> +             interrupt-names = "pci", "wakeup";
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&wlan_host_wake_l>;
>> +             wakeup-source;
>> +
>>               mvl_wifi: wifi@0,0 {
>>                       compatible = "pci1b4b,2b42";
>>                       reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>>                              0x83010000 0x0 0x00100000 0x0 0x00100000>;
>> -                     interrupt-parent = <&gpio0>;
>> -                     interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>> -                     pinctrl-names = "default";
>> -                     pinctrl-0 = <&wlan_host_wake_l>;
>> -                     wakeup-source;
>>               };
>>       };
>>  };
>
> So the above modifies pcie@0,0 node. And that node describes
> the particular PCIe port that the WLAN is connected to instead
> of describing the whole PCIe controller device, right?
>
> If so, then yeah it's totally where the wakeirq should be
> defined for a PCIe device in the dts file :)

As long as the convention used here is clear to everybody, that is.

Thanks,
Rafael

[RFC PATCH v12 2/5] of/irq: Adjust of_pci_irq parsing for multiple interrupts

[Jeffy Chen]

Currently we are considering the first irq as the PCI interrupt pin,
but a PCI device may have multiple interrupts(e.g. PCIe WAKE# pin).

Only parse the PCI interrupt pin when the irq is unnamed or named as
"pci".

Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

Changes in v13: None
Changes in v12: None
Changes in v11:
Address Brian's comments.

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v3: None
Changes in v2: None

 drivers/of/of_pci_irq.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 3a05568f65df..d39565d5477b 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -27,9 +27,25 @@ int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq
 	 */
 	dn = pci_device_to_OF_node(pdev);
 	if (dn) {
-		rc = of_irq_parse_one(dn, 0, out_irq);
-		if (!rc)
-			return rc;
+		struct property *prop;
+		const char *name;
+		int index = 0;
+
+		of_property_for_each_string(dn, "interrupt-names", prop, name) {
+			if (!strcmp(name, "pci"))
+				break;
+			index++;
+		}
+
+		/*
+		 * Only parse from DT if we have no "interrupt-names",
+		 * or if we found an interrupt named "pci".
+		 */
+		if (index == 0 || name) {
+			rc = of_irq_parse_one(dn, index, out_irq);
+			if (!rc)
+				return rc;
+		}
 	}
 
 	/* Ok, we don't, time to have fun. Let's start by building up an
-- 
2.11.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 2/5] of/irq: Adjust of_pci_irq parsing for multiple interrupts

[Rob Herring]

On Mon, Dec 25, 2017 at 8:36 PM, Jeffy Chen <jeffy.chen@rock-chips.com> wrote:
> Currently we are considering the first irq as the PCI interrupt pin,
> but a PCI device may have multiple interrupts(e.g. PCIe WAKE# pin).
>
> Only parse the PCI interrupt pin when the irq is unnamed or named as
> "pci".
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v13: None
> Changes in v12: None
> Changes in v11:
> Address Brian's comments.
>
> Changes in v10: None
> Changes in v9: None
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v3: None
> Changes in v2: None
>
>  drivers/of/of_pci_irq.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
> index 3a05568f65df..d39565d5477b 100644
> --- a/drivers/of/of_pci_irq.c
> +++ b/drivers/of/of_pci_irq.c
> @@ -27,9 +27,25 @@ int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq
>          */
>         dn = pci_device_to_OF_node(pdev);
>         if (dn) {
> -               rc = of_irq_parse_one(dn, 0, out_irq);
> -               if (!rc)
> -                       return rc;
> +               struct property *prop;
> +               const char *name;
> +               int index = 0;
> +
> +               of_property_for_each_string(dn, "interrupt-names", prop, name) {
> +                       if (!strcmp(name, "pci"))
> +                               break;
> +                       index++;
> +               }
> +
> +               /*
> +                * Only parse from DT if we have no "interrupt-names",
> +                * or if we found an interrupt named "pci".
> +                */
> +               if (index == 0 || name) {
> +                       rc = of_irq_parse_one(dn, index, out_irq);
> +                       if (!rc)
> +                               return rc;
> +               }

As mentioned before, use of_property_match_string. The following should work:

index = of_property_match_string(dn, "interrupt-names", "pci");
if (index == -EINVAL)  /* Property doesn't exist */
  index = 0;

if (index >= 0) {
  rc = of_irq_parse_one(dn, index, out_irq);
  if (!rc)
    return rc;
}


>         }
>
>         /* Ok, we don't, time to have fun. Let's start by building up an
> --
> 2.11.0
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 2/5] of/irq: Adjust of_pci_irq parsing for multiple interrupts

[JeffyChen]

Hi Rob,

Thanks for your reply :)

On 12/26/2017 11:48 PM, Rob Herring wrote:
>> >+               /*
>> >+                * Only parse from DT if we have no "interrupt-names",
>> >+                * or if we found an interrupt named "pci".
>> >+                */
>> >+               if (index == 0 || name) {
>> >+                       rc = of_irq_parse_one(dn, index, out_irq);
>> >+                       if (!rc)
>> >+                               return rc;
>> >+               }
> As mentioned before, use of_property_match_string. The following should work:
>
> index = of_property_match_string(dn, "interrupt-names", "pci");
> if (index == -EINVAL)  /* Property doesn't exist */
>    index = 0;
>
> if (index >= 0) {
>    rc = of_irq_parse_one(dn, index, out_irq);
>    if (!rc)
>      return rc;
> }

hmmm, right, that should work. didn't notice of_property_match_string() 
would return different error numbers for string not found and property 
doesn't exist...

will do this in the next version, thanks :)

>
>

[RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Jeffy Chen]

Add of_pci_setup_wake_irq() and of_pci_teardown_wake_irq() to handle
the PCIe WAKE# interrupt.

Also use the dedicated wakeirq infrastructure to simplify it.

And add pci-of.c to enable/disable the wakeup irq in noirq stage to
avoid possible irq storm.

Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

Changes in v13:
Fix compiler error reported by kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Changes in v12:
Enable the wake irq in noirq stage to avoid possible irq storm.

Changes in v11:
Only support 1-per-device PCIe WAKE# pin as suggested.

Changes in v10:
Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
since dedicated wakeirq will be lost in device_set_wakeup_enable(false).

Changes in v9:
Fix check error in .cleanup().
Move dedicated wakeirq setup to setup() callback and use
device_set_wakeup_enable() to enable/disable.

Changes in v8:
Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.

Changes in v7:
Move PCIE_WAKE handling into pci core.

Changes in v6:
Fix device_init_wake error handling, and add some comments.

Changes in v5:
Rebase.

Changes in v3:
Fix error handling.

Changes in v2:
Use dev_pm_set_dedicated_wake_irq.

 drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++
 drivers/pci/Makefile     |  1 +
 drivers/pci/pci-driver.c | 10 +++++++
 drivers/pci/pci-of.c     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_pci.h   |  9 ++++++
 5 files changed, 144 insertions(+)
 create mode 100644 drivers/pci/pci-of.c

diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index d39565d5477b..abec3a44853b 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -1,8 +1,57 @@
 #include <linux/kernel.h>
 #include <linux/of_pci.h>
 #include <linux/of_irq.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/export.h>
 
+int of_pci_setup_wake_irq(struct pci_dev *pdev)
+{
+	struct pci_dev *ppdev;
+	struct device_node *dn;
+	int ret, irq;
+
+	/* Get the pci_dev of our parent. Hopefully it's a port. */
+	ppdev = pdev->bus->self;
+	/* Nope, it's a host bridge. */
+	if (!ppdev)
+		return 0;
+
+	dn = pci_device_to_OF_node(ppdev);
+	if (!dn)
+		return 0;
+
+	irq = of_irq_get_byname(dn, "wakeup");
+	if (irq == -EPROBE_DEFER) {
+		return irq;
+	} else if (irq < 0) {
+		/* Ignore other errors, since a missing wakeup is non-fatal. */
+		dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
+		return 0;
+	}
+
+	device_init_wakeup(&pdev->dev, true);
+
+	ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, irq);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to set wake IRQ: %d\n", ret);
+		device_init_wakeup(&pdev->dev, false);
+		return ret;
+	}
+
+	/* Start out disabled to avoid irq storm */
+	dev_pm_disable_wake_irq(&pdev->dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq);
+
+void of_pci_teardown_wake_irq(struct pci_dev *pdev)
+{
+	dev_pm_clear_wake_irq(&pdev->dev);
+	device_init_wakeup(&pdev->dev, false);
+}
+EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq);
+
 /**
  * of_irq_parse_pci - Resolve the interrupt for a PCI device
  * @pdev:       the device whose interrupt is to be resolved
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index c7819b973df7..d0182c82162a 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
 # ACPI _DSM provided firmware instance and string name
 #
 obj-$(CONFIG_ACPI)    += pci-acpi.o
+obj-$(CONFIG_OF)      += pci-of.o
 
 # SMBIOS provided firmware instance and labels
 obj-$(CONFIG_PCI_LABEL) += pci-label.o
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index d79dbc377b9c..b4475ff35d97 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/cpu.h>
+#include <linux/of_pci.h>
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
 #include <linux/kexec.h>
@@ -421,10 +422,17 @@ static int pci_device_probe(struct device *dev)
 	if (error < 0)
 		return error;
 
+	error = of_pci_setup_wake_irq(pci_dev);
+	if (error < 0) {
+		pcibios_free_irq(pci_dev);
+		return error;
+	}
+
 	pci_dev_get(pci_dev);
 	if (pci_device_can_probe(pci_dev)) {
 		error = __pci_device_probe(drv, pci_dev);
 		if (error) {
+			of_pci_teardown_wake_irq(pci_dev);
 			pcibios_free_irq(pci_dev);
 			pci_dev_put(pci_dev);
 		}
@@ -438,6 +446,8 @@ static int pci_device_remove(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct pci_driver *drv = pci_dev->driver;
 
+	of_pci_teardown_wake_irq(pci_dev);
+
 	if (drv) {
 		if (drv->remove) {
 			pm_runtime_get_sync(dev);
diff --git a/drivers/pci/pci-of.c b/drivers/pci/pci-of.c
new file mode 100644
index 000000000000..ad413b2de508
--- /dev/null
+++ b/drivers/pci/pci-of.c
@@ -0,0 +1,75 @@
+/*
+ * File:	pci-of.c
+ * Purpose:	Provide PCI PM/wakeup support in OF systems
+ *
+ * Copyright (C) 2017 Google, Inc.
+ *	Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
+ *
+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
+ *	Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pm_wakeirq.h>
+#include "pci.h"
+
+static bool of_pci_power_manageable(struct pci_dev *dev)
+{
+	return false;
+}
+
+static int of_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+	return -ENOSYS;
+}
+
+static pci_power_t of_pci_get_power_state(struct pci_dev *dev)
+{
+	return PCI_UNKNOWN;
+}
+
+static pci_power_t of_pci_choose_state(struct pci_dev *pdev)
+{
+	return PCI_POWER_ERROR;
+}
+
+static int of_pci_wakeup(struct pci_dev *dev, bool enable)
+{
+	if (enable)
+		dev_pm_enable_wake_irq(&dev->dev);
+	else
+		dev_pm_disable_wake_irq(&dev->dev);
+	return 0;
+}
+
+static bool of_pci_need_resume(struct pci_dev *dev)
+{
+	return false;
+}
+
+static const struct pci_platform_pm_ops of_pci_platform_pm = {
+	.is_manageable = of_pci_power_manageable,
+	.set_state = of_pci_set_power_state,
+	.get_state = of_pci_get_power_state,
+	.choose_state = of_pci_choose_state,
+	.set_wakeup = of_pci_wakeup,
+	.need_resume = of_pci_need_resume,
+};
+
+static int __init of_pci_init(void)
+{
+	if (!acpi_disabled)
+		return 0;
+
+	pci_set_platform_pm(&of_pci_platform_pm);
+
+	return 0;
+}
+arch_initcall(of_pci_init);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bf588a05d0d0..80fe8ae3393e 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,6 +10,8 @@ struct of_phandle_args;
 struct device_node;
 
 #ifdef CONFIG_OF_PCI
+int of_pci_setup_wake_irq(struct pci_dev *pdev);
+void of_pci_teardown_wake_irq(struct pci_dev *pdev);
 int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq);
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
@@ -23,6 +25,13 @@ int of_pci_map_rid(struct device_node *np, u32 rid,
 		   const char *map_name, const char *map_mask_name,
 		   struct device_node **target, u32 *id_out);
 #else
+static inline int of_pci_setup_wake_irq(struct pci_dev *pdev)
+{
+	return 0;
+}
+
+static void of_pci_teardown_wake_irq(struct pci_dev *pdev) { };
+
 static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
 {
 	return 0;
-- 
2.11.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Rob Herring]

On Tue, Dec 26, 2017 at 10:36:45AM +0800, Jeffy Chen wrote:
> Add of_pci_setup_wake_irq() and of_pci_teardown_wake_irq() to handle
> the PCIe WAKE# interrupt.
> 
> Also use the dedicated wakeirq infrastructure to simplify it.
> 
> And add pci-of.c to enable/disable the wakeup irq in noirq stage to
> avoid possible irq storm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
> Changes in v13:
> Fix compiler error reported by kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Changes in v12:
> Enable the wake irq in noirq stage to avoid possible irq storm.
> 
> Changes in v11:
> Only support 1-per-device PCIe WAKE# pin as suggested.
> 
> Changes in v10:
> Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
> since dedicated wakeirq will be lost in device_set_wakeup_enable(false).
> 
> Changes in v9:
> Fix check error in .cleanup().
> Move dedicated wakeirq setup to setup() callback and use
> device_set_wakeup_enable() to enable/disable.
> 
> Changes in v8:
> Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
> 
> Changes in v7:
> Move PCIE_WAKE handling into pci core.
> 
> Changes in v6:
> Fix device_init_wake error handling, and add some comments.
> 
> Changes in v5:
> Rebase.
> 
> Changes in v3:
> Fix error handling.
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq.
> 
>  drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++

Please move this to drivers/pci/of.c (or perhaps create pci/of_irq.c).

>  drivers/pci/Makefile     |  1 +
>  drivers/pci/pci-driver.c | 10 +++++++
>  drivers/pci/pci-of.c     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++

We already have drivers/pci/of.c. It's not clear what the difference is 
from the filenames. Either merge with of.c or perhaps of-pm.c.

>  include/linux/of_pci.h   |  9 ++++++
>  5 files changed, 144 insertions(+)
>  create mode 100644 drivers/pci/pci-of.c
> 
> diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
> index d39565d5477b..abec3a44853b 100644
> --- a/drivers/of/of_pci_irq.c
> +++ b/drivers/of/of_pci_irq.c
> @@ -1,8 +1,57 @@
>  #include <linux/kernel.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_irq.h>
> +#include <linux/pm_wakeirq.h>
>  #include <linux/export.h>
>  
> +int of_pci_setup_wake_irq(struct pci_dev *pdev)
> +{
> +	struct pci_dev *ppdev;
> +	struct device_node *dn;
> +	int ret, irq;
> +
> +	/* Get the pci_dev of our parent. Hopefully it's a port. */
> +	ppdev = pdev->bus->self;
> +	/* Nope, it's a host bridge. */
> +	if (!ppdev)
> +		return 0;
> +
> +	dn = pci_device_to_OF_node(ppdev);
> +	if (!dn)
> +		return 0;
> +
> +	irq = of_irq_get_byname(dn, "wakeup");
> +	if (irq == -EPROBE_DEFER) {
> +		return irq;
> +	} else if (irq < 0) {
> +		/* Ignore other errors, since a missing wakeup is non-fatal. */
> +		dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
> +		return 0;
> +	}
> +
> +	device_init_wakeup(&pdev->dev, true);
> +
> +	ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, irq);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to set wake IRQ: %d\n", ret);
> +		device_init_wakeup(&pdev->dev, false);
> +		return ret;
> +	}
> +
> +	/* Start out disabled to avoid irq storm */
> +	dev_pm_disable_wake_irq(&pdev->dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq);
> +
> +void of_pci_teardown_wake_irq(struct pci_dev *pdev)
> +{
> +	dev_pm_clear_wake_irq(&pdev->dev);
> +	device_init_wakeup(&pdev->dev, false);
> +}
> +EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq);
> +
>  /**
>   * of_irq_parse_pci - Resolve the interrupt for a PCI device
>   * @pdev:       the device whose interrupt is to be resolved
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index c7819b973df7..d0182c82162a 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # ACPI _DSM provided firmware instance and string name
>  #
>  obj-$(CONFIG_ACPI)    += pci-acpi.o
> +obj-$(CONFIG_OF)      += pci-of.o
>  
>  # SMBIOS provided firmware instance and labels
>  obj-$(CONFIG_PCI_LABEL) += pci-label.o
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index d79dbc377b9c..b4475ff35d97 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -17,6 +17,7 @@
>  #include <linux/slab.h>
>  #include <linux/sched.h>
>  #include <linux/cpu.h>
> +#include <linux/of_pci.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/suspend.h>
>  #include <linux/kexec.h>
> @@ -421,10 +422,17 @@ static int pci_device_probe(struct device *dev)
>  	if (error < 0)
>  		return error;
>  
> +	error = of_pci_setup_wake_irq(pci_dev);
> +	if (error < 0) {
> +		pcibios_free_irq(pci_dev);
> +		return error;
> +	}
> +
>  	pci_dev_get(pci_dev);
>  	if (pci_device_can_probe(pci_dev)) {
>  		error = __pci_device_probe(drv, pci_dev);
>  		if (error) {
> +			of_pci_teardown_wake_irq(pci_dev);
>  			pcibios_free_irq(pci_dev);
>  			pci_dev_put(pci_dev);
>  		}
> @@ -438,6 +446,8 @@ static int pci_device_remove(struct device *dev)
>  	struct pci_dev *pci_dev = to_pci_dev(dev);
>  	struct pci_driver *drv = pci_dev->driver;
>  
> +	of_pci_teardown_wake_irq(pci_dev);
> +
>  	if (drv) {
>  		if (drv->remove) {
>  			pm_runtime_get_sync(dev);
> diff --git a/drivers/pci/pci-of.c b/drivers/pci/pci-of.c
> new file mode 100644
> index 000000000000..ad413b2de508
> --- /dev/null
> +++ b/drivers/pci/pci-of.c
> @@ -0,0 +1,75 @@
> +/*
> + * File:	pci-of.c
> + * Purpose:	Provide PCI PM/wakeup support in OF systems
> + *
> + * Copyright (C) 2017 Google, Inc.
> + *	Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + *	Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/pm_wakeirq.h>
> +#include "pci.h"
> +
> +static bool of_pci_power_manageable(struct pci_dev *dev)
> +{
> +	return false;
> +}
> +
> +static int of_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
> +{
> +	return -ENOSYS;
> +}
> +
> +static pci_power_t of_pci_get_power_state(struct pci_dev *dev)
> +{
> +	return PCI_UNKNOWN;
> +}
> +
> +static pci_power_t of_pci_choose_state(struct pci_dev *pdev)
> +{
> +	return PCI_POWER_ERROR;
> +}
> +
> +static int of_pci_wakeup(struct pci_dev *dev, bool enable)
> +{
> +	if (enable)
> +		dev_pm_enable_wake_irq(&dev->dev);
> +	else
> +		dev_pm_disable_wake_irq(&dev->dev);
> +	return 0;
> +}
> +
> +static bool of_pci_need_resume(struct pci_dev *dev)
> +{
> +	return false;
> +}
> +
> +static const struct pci_platform_pm_ops of_pci_platform_pm = {
> +	.is_manageable = of_pci_power_manageable,
> +	.set_state = of_pci_set_power_state,
> +	.get_state = of_pci_get_power_state,
> +	.choose_state = of_pci_choose_state,
> +	.set_wakeup = of_pci_wakeup,
> +	.need_resume = of_pci_need_resume,
> +};
> +
> +static int __init of_pci_init(void)
> +{
> +	if (!acpi_disabled)
> +		return 0;
> +
> +	pci_set_platform_pm(&of_pci_platform_pm);

I guess no DT based system will override this?

> +
> +	return 0;
> +}
> +arch_initcall(of_pci_init);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[JeffyChen]

Hi Rob,

Thanks for your reply.

On 12/27/2017 07:56 AM, Rob Herring wrote:
>> >
>> >  drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++
> Please move this to drivers/pci/of.c (or perhaps create pci/of_irq.c).
>
>> >  drivers/pci/Makefile     |  1 +
>> >  drivers/pci/pci-driver.c | 10 +++++++
>> >  drivers/pci/pci-of.c     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
> We already have drivers/pci/of.c. It's not clear what the difference is
> from the filenames. Either merge with of.c or perhaps of-pm.c.

this new file does something similar to the pci-acpi.c and pci-mid.c..

and i am agree the naming is not clear, maybe we can rename both of 
those files to something like pci-pm-***.c?

Hi Rafael, do you think this would make sense?

>> >  include/linux/of_pci.h   |  9 ++++++
>> >  5 files changed, 144 insertions(+)
>> >  create mode 100644 drivers/pci/pci-of.c
>> >
>> >diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
>> >index d39565d5477b..abec3a44853b 100644
>> >--- a/drivers/of/of_pci_irq.c
>> >+++ b/drivers/of/of_pci_irq.c
>> >@@ -1,8 +1,57 @@
>> >  #include <linux/kernel.h>
>> >  #include <linux/of_pci.h>
>> >  #include <linux/of_irq.h>
>> >+#include <linux/pm_wakeirq.h>
>> >  #include <linux/export.h>
>> >
>> >+int of_pci_setup_wake_irq(struct pci_dev *pdev)
>> >+{
>> >+	struct pci_dev *ppdev;
>> >+	struct device_node *dn;
>> >+	int ret, irq;
>> >+
>> >+	/* Get the pci_dev of our parent. Hopefully it's a port. */
>> >+	ppdev = pdev->bus->self;
>> >+	/* Nope, it's a host bridge. */
>> >+	if (!ppdev)
>> >+		return 0;
>> >+
>> >+	dn = pci_device_to_OF_node(ppdev);
>> >+	if (!dn)
>> >+		return 0;
>> >+
>> >+	irq = of_irq_get_byname(dn, "wakeup");
>> >+	if (irq == -EPROBE_DEFER) {
>> >+		return irq;
>> >+	} else if (irq < 0) {
>> >+		/* Ignore other errors, since a missing wakeup is non-fatal. */
>> >+		dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
>> >+		return 0;
>> >+	}
>> >+
>> >+	device_init_wakeup(&pdev->dev, true);
>> >+
>> >+	ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, irq);
>> >+	if (ret < 0) {
>> >+		dev_err(&pdev->dev, "failed to set wake IRQ: %d\n", ret);
>> >+		device_init_wakeup(&pdev->dev, false);
>> >+		return ret;
>> >+	}
>> >+
>> >+	/* Start out disabled to avoid irq storm */
>> >+	dev_pm_disable_wake_irq(&pdev->dev);
>> >+
>> >+	return 0;
>> >+}
>> >+EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq);
>> >+
>> >+void of_pci_teardown_wake_irq(struct pci_dev *pdev)
>> >+{
>> >+	dev_pm_clear_wake_irq(&pdev->dev);
>> >+	device_init_wakeup(&pdev->dev, false);
>> >+}
>> >+EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq);
>> >+
>> >  /**
>> >   * of_irq_parse_pci - Resolve the interrupt for a PCI device
>> >   * @pdev:       the device whose interrupt is to be resolved
>> >diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>> >index c7819b973df7..d0182c82162a 100644
>> >--- a/drivers/pci/Makefile
>> >+++ b/drivers/pci/Makefile
>> >@@ -29,6 +29,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>> >  # ACPI _DSM provided firmware instance and string name
>> >  #
>> >  obj-$(CONFIG_ACPI)    += pci-acpi.o
>> >+obj-$(CONFIG_OF)      += pci-of.o
>> >
>> >  # SMBIOS provided firmware instance and labels
>> >  obj-$(CONFIG_PCI_LABEL) += pci-label.o
>> >diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> >index d79dbc377b9c..b4475ff35d97 100644
>> >--- a/drivers/pci/pci-driver.c
>> >+++ b/drivers/pci/pci-driver.c
>> >@@ -17,6 +17,7 @@
>> >  #include <linux/slab.h>
>> >  #include <linux/sched.h>
>> >  #include <linux/cpu.h>
>> >+#include <linux/of_pci.h>
>> >  #include <linux/pm_runtime.h>
>> >  #include <linux/suspend.h>
>> >  #include <linux/kexec.h>
>> >@@ -421,10 +422,17 @@ static int pci_device_probe(struct device *dev)
>> >  	if (error < 0)
>> >  		return error;
>> >
>> >+	error = of_pci_setup_wake_irq(pci_dev);
>> >+	if (error < 0) {
>> >+		pcibios_free_irq(pci_dev);
>> >+		return error;
>> >+	}
>> >+
>> >  	pci_dev_get(pci_dev);
>> >  	if (pci_device_can_probe(pci_dev)) {
>> >  		error = __pci_device_probe(drv, pci_dev);
>> >  		if (error) {
>> >+			of_pci_teardown_wake_irq(pci_dev);
>> >  			pcibios_free_irq(pci_dev);
>> >  			pci_dev_put(pci_dev);
>> >  		}
>> >@@ -438,6 +446,8 @@ static int pci_device_remove(struct device *dev)
>> >  	struct pci_dev *pci_dev = to_pci_dev(dev);
>> >  	struct pci_driver *drv = pci_dev->driver;
>> >
>> >+	of_pci_teardown_wake_irq(pci_dev);
>> >+
>> >  	if (drv) {
>> >  		if (drv->remove) {
>> >  			pm_runtime_get_sync(dev);
>> >diff --git a/drivers/pci/pci-of.c b/drivers/pci/pci-of.c
>> >new file mode 100644
>> >index 000000000000..ad413b2de508
>> >--- /dev/null
>> >+++ b/drivers/pci/pci-of.c
>> >@@ -0,0 +1,75 @@
>> >+/*
>> >+ * File:	pci-of.c
>> >+ * Purpose:	Provide PCI PM/wakeup support in OF systems
>> >+ *
>> >+ * Copyright (C) 2017 Google, Inc.
>> >+ *	Brian Norris<briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> >+ *
>> >+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> >+ *	Jeffy Chen<jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> >+ *
>> >+ * This program is free software; you can redistribute it and/or modify it
>> >+ * under the terms and conditions of the GNU General Public License,
>> >+ * version 2, as published by the Free Software Foundation.
>> >+ */
>> >+
>> >+#include <linux/acpi.h>
>> >+#include <linux/init.h>
>> >+#include <linux/module.h>
>> >+#include <linux/pci.h>
>> >+#include <linux/pm_wakeirq.h>
>> >+#include "pci.h"
>> >+
>> >+static bool of_pci_power_manageable(struct pci_dev *dev)
>> >+{
>> >+	return false;
>> >+}
>> >+
>> >+static int of_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
>> >+{
>> >+	return -ENOSYS;
>> >+}
>> >+
>> >+static pci_power_t of_pci_get_power_state(struct pci_dev *dev)
>> >+{
>> >+	return PCI_UNKNOWN;
>> >+}
>> >+
>> >+static pci_power_t of_pci_choose_state(struct pci_dev *pdev)
>> >+{
>> >+	return PCI_POWER_ERROR;
>> >+}
>> >+
>> >+static int of_pci_wakeup(struct pci_dev *dev, bool enable)
>> >+{
>> >+	if (enable)
>> >+		dev_pm_enable_wake_irq(&dev->dev);
>> >+	else
>> >+		dev_pm_disable_wake_irq(&dev->dev);
>> >+	return 0;
>> >+}
>> >+
>> >+static bool of_pci_need_resume(struct pci_dev *dev)
>> >+{
>> >+	return false;
>> >+}
>> >+
>> >+static const struct pci_platform_pm_ops of_pci_platform_pm = {
>> >+	.is_manageable = of_pci_power_manageable,
>> >+	.set_state = of_pci_set_power_state,
>> >+	.get_state = of_pci_get_power_state,
>> >+	.choose_state = of_pci_choose_state,
>> >+	.set_wakeup = of_pci_wakeup,
>> >+	.need_resume = of_pci_need_resume,
>> >+};
>> >+
>> >+static int __init of_pci_init(void)
>> >+{
>> >+	if (!acpi_disabled)
>> >+		return 0;
>> >+
>> >+	pci_set_platform_pm(&of_pci_platform_pm);
> I guess no DT based system will override this?

i think the !acpi_disabled means acpi been disabled or CONFIG_ACPI is 
undefined.

and pci-acpi.c would only work when we have CONFIG_ACPI.

but i have no idea about pci-mid.c or would it possible to have more 
platform pm ops in the future...maybe we should add some dependency in 
the Kconfig?

>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Rob Herring]

On Tue, Dec 26, 2017 at 7:32 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Hi Rob,
>
> Thanks for your reply.
>
> On 12/27/2017 07:56 AM, Rob Herring wrote:
>>>
>>> >
>>> >  drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++
>>
>> Please move this to drivers/pci/of.c (or perhaps create pci/of_irq.c).
>>
>>> >  drivers/pci/Makefile     |  1 +
>>> >  drivers/pci/pci-driver.c | 10 +++++++
>>> >  drivers/pci/pci-of.c     | 75
>>> > ++++++++++++++++++++++++++++++++++++++++++++++++
>>
>> We already have drivers/pci/of.c. It's not clear what the difference is
>> from the filenames. Either merge with of.c or perhaps of-pm.c.
>
>
> this new file does something similar to the pci-acpi.c and pci-mid.c..

pci-acpi.c has similar things to pci/of.c. The naming is just not consistent.

Also, I plan to move the rest of drivers/of/of_pci* to drivers/pci.

> and i am agree the naming is not clear, maybe we can rename both of those
> files to something like pci-pm-***.c?

At least pci-acpi.c is more than just PM functions, so that doesn't
make sense. Given that all the ACPI related functions are in 1 file,
we should do the same for DT.

>
> Hi Rafael, do you think this would make sense?
>
>

[...]

>>> >+static int __init of_pci_init(void)
>>> >+{
>>> >+      if (!acpi_disabled)
>>> >+              return 0;
>>> >+
>>> >+      pci_set_platform_pm(&of_pci_platform_pm);
>>
>> I guess no DT based system will override this?
>
>
> i think the !acpi_disabled means acpi been disabled or CONFIG_ACPI is
> undefined.
>
> and pci-acpi.c would only work when we have CONFIG_ACPI.
>
> but i have no idea about pci-mid.c or would it possible to have more
> platform pm ops in the future...maybe we should add some dependency in the
> Kconfig?

It's probably fine given there are only 2 other implementations so far.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Rafael J. Wysocki]

On Wed, Dec 27, 2017 at 4:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Dec 26, 2017 at 7:32 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> Hi Rob,
>>
>> Thanks for your reply.
>>
>> On 12/27/2017 07:56 AM, Rob Herring wrote:
>>>>
>>>> >
>>>> >  drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++
>>>
>>> Please move this to drivers/pci/of.c (or perhaps create pci/of_irq.c).
>>>
>>>> >  drivers/pci/Makefile     |  1 +
>>>> >  drivers/pci/pci-driver.c | 10 +++++++
>>>> >  drivers/pci/pci-of.c     | 75
>>>> > ++++++++++++++++++++++++++++++++++++++++++++++++
>>>
>>> We already have drivers/pci/of.c. It's not clear what the difference is
>>> from the filenames. Either merge with of.c or perhaps of-pm.c.
>>
>>
>> this new file does something similar to the pci-acpi.c and pci-mid.c..
>
> pci-acpi.c has similar things to pci/of.c. The naming is just not consistent.
>
> Also, I plan to move the rest of drivers/of/of_pci* to drivers/pci.
>
>> and i am agree the naming is not clear, maybe we can rename both of those
>> files to something like pci-pm-***.c?
>
> At least pci-acpi.c is more than just PM functions, so that doesn't
> make sense. Given that all the ACPI related functions are in 1 file,
> we should do the same for DT.

Agreed.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Andy Shevchenko]

On Wed, 2017-12-27 at 09:30 -0600, Rob Herring wrote:
> On Tue, Dec 26, 2017 at 7:32 PM, JeffyChen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> wrote:

> this new file does something similar to the pci-acpi.c and pci-mid.c..

> pci-acpi.c has similar things to pci/of.c. The naming is just not
> consistent.

> > and i am agree the naming is not clear, maybe we can rename both of
> > those
> > files to something like pci-pm-***.c?
> 
> At least pci-acpi.c is more than just PM functions, so that doesn't
> make sense. Given that all the ACPI related functions are in 1 file,
> we should do the same for DT.

> > but i have no idea about pci-mid.c or would it possible to have more
> > platform pm ops in the future...maybe we should add some dependency
> > in the
> > Kconfig?

You may consider pci-mid.c as pci-sfi.c to some extend.

So, in that sense it more looks like pci-of.c would be the name for DT
case.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core

[Rafael J. Wysocki]

On Tuesday, December 26, 2017 3:36:41 AM CET Jeffy Chen wrote:
> 
> Currently we are handling wake irq in mrvl wifi driver. Move it into
> pci core.
> 
> Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).
> 
> 
> Changes in v13:
> Fix compiler error reported by kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Changes in v12:
> Only add irq definitions for PCI devices and rewrite the commit message.
> Enable the wake irq in noirq stage to avoid possible irq storm.
> 
> Changes in v11:
> Address Brian's comments.
> Only support 1-per-device PCIe WAKE# pin as suggested.
> Move to pcie port as Brian suggested.
> 
> Changes in v10:
> Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
> since dedicated wakeirq will be lost in device_set_wakeup_enable(false).
> 
> Changes in v9:
> Add section for PCI devices and rewrite the commit message.
> Fix check error in .cleanup().
> Move dedicated wakeirq setup to setup() callback and use
> device_set_wakeup_enable() to enable/disable.
> Rewrite the commit message.
> 
> Changes in v8:
> Add optional "pci", and rewrite commit message.
> Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
> Rewrite the commit message.
> 
> Changes in v7:
> Move PCIE_WAKE handling into pci core.
> 
> Changes in v6:
> Fix device_init_wake error handling, and add some comments.
> 
> Changes in v5:
> Move to pci.txt
> Rebase.
> Use "wakeup" instead of "wake"
> 
> Changes in v3:
> Fix error handling.
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq.
> 
> Jeffy Chen (5):
>   dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
>   of/irq: Adjust of_pci_irq parsing for multiple interrupts
>   mwifiex: Disable wakeup irq handling for pcie
>   PCI / PM: Add support for the PCIe WAKE# signal for OF
>   arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru
> 
>  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 11 ++--
>  drivers/net/wireless/marvell/mwifiex/main.c   |  4 ++
>  drivers/of/of_pci_irq.c                       | 71 +++++++++++++++++++++++--
>  drivers/pci/Makefile                          |  1 +
>  drivers/pci/pci-driver.c                      | 10 ++++
>  drivers/pci/pci-of.c                          | 75 +++++++++++++++++++++++++++
>  include/linux/of_pci.h                        |  9 ++++
>  8 files changed, 183 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/pci/pci-of.c

I'm going to ignore this version till the discussion on the previous one is
over.

Thanks,
Rafael


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core

[Jeffy Chen]

Currently we are handling wake irq in mrvl wifi driver. Move it into
pci core.

Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).


Changes in v12:
Enable the wake irq in noirq stage to avoid possible irq storm.

Changes in v11:
Only add irq definitions for PCI devices and rewrite the commit message.
Address Brian's comments.
Only support 1-per-device PCIe WAKE# pin as suggested.
Move to pcie port as Brian suggested.

Changes in v10:
Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
since dedicated wakeirq will be lost in device_set_wakeup_enable(false).

Changes in v9:
Add section for PCI devices and rewrite the commit message.
Fix check error in .cleanup().
Move dedicated wakeirq setup to setup() callback and use
device_set_wakeup_enable() to enable/disable.
Rewrite the commit message.

Changes in v8:
Add optional "pci", and rewrite commit message.
Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
Rewrite the commit message.

Changes in v7:
Move PCIE_WAKE handling into pci core.

Changes in v6:
Fix device_init_wake error handling, and add some comments.

Changes in v5:
Move to pci.txt
Rebase.
Use "wakeup" instead of "wake"

Changes in v3:
Fix error handling.

Changes in v2:
Use dev_pm_set_dedicated_wake_irq.

Jeffy Chen (5):
  dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
  of/irq: Adjust of_pci_irq parsing for multiple interrupts
  mwifiex: Disable wakeup irq handling for pcie
  PCI / PM: Add support for the PCIe WAKE# signal for OF
  arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru

 Documentation/devicetree/bindings/pci/pci.txt | 10 ++++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 11 ++--
 drivers/net/wireless/marvell/mwifiex/main.c   |  4 ++
 drivers/of/of_pci_irq.c                       | 74 ++++++++++++++++++++++++--
 drivers/pci/Makefile                          |  1 +
 drivers/pci/pci-driver.c                      | 10 ++++
 drivers/pci/pci-of.c                          | 75 +++++++++++++++++++++++++++
 include/linux/of_pci.h                        |  9 ++++
 8 files changed, 186 insertions(+), 8 deletions(-)
 create mode 100644 drivers/pci/pci-of.c

-- 
2.11.0

[RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Jeffy Chen]

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(+)

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.
-- 
2.11.0

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Tony Lindgren]

* 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
> 
> 

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Rafael J. Wysocki]

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
>>
>>

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Rafael J. Wysocki]

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

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Tony Lindgren]

* 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

Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq

[Tony Lindgren]

* 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