[RFC PATCH 0/3] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[RFC PATCH 0/3] PCI: rockchip: Move PCIE_WAKE handling into rockchip pcie driver

[Jeffy Chen]

Currently we are handling pcie wake in mrvl wifi driver. But Brian
suggests to move it into rockchip pcie driver.

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



Jeffy Chen (3):
  PCI: rockchip: Add support for pcie wake irq
  dt-bindings: PCI: rockchip: Add support for pcie wake irq
  arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi       | 17 ++++---
 drivers/pci/host/pcie-rockchip.c                   | 58 ++++++++++++++++++++++
 3 files changed, 76 insertions(+), 10 deletions(-)

-- 
2.11.0

[RFC PATCH 2/3] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Jeffy Chen]

Add an optional interrupt for PCIE_WAKE pin.

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

 Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index 1453a734c2f5..6ef9903567db 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -22,10 +22,13 @@ Required properties:
 - phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
 - phy-names:  MUST be "pcie-phy".
 - interrupts: Three interrupt entries must be specified.
-- interrupt-names: Must include the following names
-	- "sys"
-	- "legacy"
-	- "client"
+- interrupt-names: Include the following names
+	Required:
+		- "sys"
+		- "legacy"
+		- "client"
+	Optional:
+		- "wake"
 - resets: Must contain seven entries for each entry in reset-names.
 	   See ../reset/reset.txt for details.
 - reset-names: Must include the following names
-- 
2.11.0

[RFC PATCH 3/3] arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

[Jeffy Chen]

Currently we are handling pcie wake irq in mrvl wifi driver.
Move it to rockchip pcie driver for Gru boards.

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

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index d48e98b62d09..7a5e1517a496 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -712,11 +712,21 @@ ap_i2c_audio: &i2c8 {
 
 	ep-gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wifi_perst_l>;
+	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wlan_host_wake_l>, <&wifi_perst_l>;
+
+	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
+			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
+	interrupt-names = "sys", "legacy", "client", "wake";
+	/delete-property/ interrupts;
+
 	vpcie3v3-supply = <&pp3300_wifi_bt>;
 	vpcie1v8-supply = <&wlan_pd_n>; /* HACK: see &wlan_pd_n */
 	vpcie0v9-supply = <&pp900_pcie>;
 
+	wakeup-source;
+
 	pci_rootport: pcie@0,0 {
 		reg = <0x83000000 0x0 0x00000000 0x0 0x00000000>;
 		#address-cells = <3>;
@@ -727,11 +737,6 @@ ap_i2c_audio: &i2c8 {
 			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 3/3] arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

[Shawn Lin]

Hi Jeffy,

On 2017/8/16 15:52, Jeffy Chen wrote:
> Currently we are handling pcie wake irq in mrvl wifi driver.
> Move it to rockchip pcie driver for Gru boards.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
>   arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> index d48e98b62d09..7a5e1517a496 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -712,11 +712,21 @@ ap_i2c_audio: &i2c8 {
>   
>   	ep-gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>;
>   	pinctrl-names = "default";
> -	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wifi_perst_l>;
> +	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wlan_host_wake_l>, <&wifi_perst_l>;
> +
> +	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
> +			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> +	interrupt-names = "sys", "legacy", "client", "wake";
> +	/delete-property/ interrupts;
> +

Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
says "Nodes that describe devices which generate interrupts must contain
an "interrupts" property, an "interrupts-extended" property, or both. If
both are present, the latter should take precedence;"

But it doesn't seem real as drivers/of/irq.c actually says "Try the
new-style interrupts-extended first"

Anyway, it seems you could safely add interrupts-extended and don't need
to delete "interrupts".

>   	vpcie3v3-supply = <&pp3300_wifi_bt>;
>   	vpcie1v8-supply = <&wlan_pd_n>; /* HACK: see &wlan_pd_n */
>   	vpcie0v9-supply = <&pp900_pcie>;
>   
> +	wakeup-source;
> +

I also have a question for this one but will inline that in the patch 1

>   	pci_rootport: pcie@0,0 {
>   		reg = <0x83000000 0x0 0x00000000 0x0 0x00000000>;
>   		#address-cells = <3>;
> @@ -727,11 +737,6 @@ ap_i2c_audio: &i2c8 {
>   			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;
>   		};
>   	};
>   };
> 

--
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 2/3] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[Shawn Lin]

Hi Jeffy

On 2017/8/16 15:52, Jeffy Chen wrote:
> Add an optional interrupt for PCIE_WAKE pin.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
>   Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> index 1453a734c2f5..6ef9903567db 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -22,10 +22,13 @@ Required properties:
>   - phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
>   - phy-names:  MUST be "pcie-phy".
>   - interrupts: Three interrupt entries must be specified.
> -- interrupt-names: Must include the following names
> -	- "sys"
> -	- "legacy"
> -	- "client"
> +- interrupt-names: Include the following names
> +	Required:
> +		- "sys"
> +		- "legacy"
> +		- "client"
> +	Optional:
> +		- "wake"

It would be better to introduce interrupts-extended and show a example
here. :)

>   - resets: Must contain seven entries for each entry in reset-names.
>   	   See ../reset/reset.txt for details.
>   - reset-names: Must include the following names
> 

--
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 3/3] arm64: dts: rockchip: Handle pcie wake in pcie driver for Gru

[Brian Norris]

Hi Shawn,

On Wed, Aug 16, 2017 at 04:33:38PM +0800, Shawn Lin wrote:
> On 2017/8/16 15:52, Jeffy Chen wrote:
> >Currently we are handling pcie wake irq in mrvl wifi driver.
> >Move it to rockchip pcie driver for Gru boards.
> >
> >Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >---
> >
> >  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> >diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> >index d48e98b62d09..7a5e1517a496 100644
> >--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> >+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> >@@ -712,11 +712,21 @@ ap_i2c_audio: &i2c8 {
> >  	ep-gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>;
> >  	pinctrl-names = "default";
> >-	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wifi_perst_l>;
> >+	pinctrl-0 = <&pcie_clkreqn_cpm>, <&wlan_host_wake_l>, <&wifi_perst_l>;
> >+
> >+	interrupts-extended = <&gic GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH 0>,
> >+			      <&gic GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH 0>,
> >+			      <&gic GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH 0>,
> >+			      <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> >+	interrupt-names = "sys", "legacy", "client", "wake";
> >+	/delete-property/ interrupts;
> >+
> 
> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> says "Nodes that describe devices which generate interrupts must contain
> an "interrupts" property, an "interrupts-extended" property, or both. If
> both are present, the latter should take precedence;"

I added that language, and as the text notes, it's just for
compatibility in case some software doesn't understand the 'extended'
property. It's occasionally useful, but not extremely often. (In my case
at that time: we actually had the DTB stuck in the bootloader, and so
would support various combinations of old/new DTBs/kernels which may or
may not understand 'interrupts-extended'.)

> But it doesn't seem real as drivers/of/irq.c actually says "Try the
> new-style interrupts-extended first"

How is that not real? That follows exactly what the binding says; "the
latter" (i.e., the new-style interrupts-extended) is taking precedence.

> Anyway, it seems you could safely add interrupts-extended and don't need
> to delete "interrupts".

You could do this, yes. Unless you really need the compatibility (and
are prepared for such a system to ignore the "wake" interrupt in that
case), I'm not sure the value in that though, as it would just make
things more confusing.

Brian
--
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 2/3] dt-bindings: PCI: rockchip: Add support for pcie wake irq

[jeffy]

Hi Shawn,

On 08/16/2017 04:35 PM, Shawn Lin wrote:
> Hi Jeffy
>
> On 2017/8/16 15:52, Jeffy Chen wrote:
>> Add an optional interrupt for PCIE_WAKE pin.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>>   Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 11
>> +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> index 1453a734c2f5..6ef9903567db 100644
>> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> @@ -22,10 +22,13 @@ Required properties:
>>   - phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
>>   - phy-names:  MUST be "pcie-phy".
>>   - interrupts: Three interrupt entries must be specified.
>> -- interrupt-names: Must include the following names
>> -    - "sys"
>> -    - "legacy"
>> -    - "client"
>> +- interrupt-names: Include the following names
>> +    Required:
>> +        - "sys"
>> +        - "legacy"
>> +        - "client"
>> +    Optional:
>> +        - "wake"
>
> It would be better to introduce interrupts-extended and show a example
> here. :)
ok, will add it in next version
>
>>   - resets: Must contain seven entries for each entry in reset-names.
>>          See ../reset/reset.txt for details.
>>   - reset-names: Must include the following names
>>
>
>
>