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

[PATCH v6 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).


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

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

Changes in v3:
Fix error handling

Changes in v2:
Use dev_pm_set_dedicated_wake_irq
        -- Suggested by Brian Norris <briannorris@chromium.com>

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

 Documentation/devicetree/bindings/pci/pci.txt |  2 ++
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 15 +++++++++------
 drivers/pci/host/pcie-rockchip.c              | 27 +++++++++++++++++++++------
 3 files changed, 32 insertions(+), 12 deletions(-)

-- 
2.11.0

[PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Jeffy Chen]

Add support for PCIE_WAKE pin in rockchip pcie driver.

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

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
        -- Suggested by Brian Norris <briannorris@chromium.com>

 drivers/pci/host/pcie-rockchip.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 9051c6c8fea4..268513b6c9c4 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -37,6 +37,7 @@
 #include <linux/pci_ids.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/reset.h>
 #include <linux/regmap.h>
 
@@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
+	irq = platform_get_irq_byname(pdev, "wakeup");
+	if (irq >= 0) {
+		/* Must init wakeup before setting dedicated wakeup irq. */
+		device_init_wakeup(dev, true);
+		err = dev_pm_set_dedicated_wake_irq(dev, irq);
+		if (err) {
+			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
+			device_init_wakeup(dev, false);
+		}
+	}
+
 	return 0;
 }
 
@@ -1123,10 +1135,6 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		return PTR_ERR(rockchip->clk_pcie_pm);
 	}
 
-	err = rockchip_pcie_setup_irq(rockchip);
-	if (err)
-		return err;
-
 	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
 	if (IS_ERR(rockchip->vpcie12v)) {
 		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
@@ -1155,7 +1163,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		dev_info(dev, "no vpcie0v9 regulator found\n");
 	}
 
-	return 0;
+	return rockchip_pcie_setup_irq(rockchip);
 }
 
 static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
@@ -1546,7 +1554,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 
 	err = rockchip_pcie_enable_clocks(rockchip);
 	if (err)
-		return err;
+		/* Cleanup rockchip_pcie_parse_dt() */
+		goto err_disable_wake;
 
 	err = rockchip_pcie_set_vpcie(rockchip);
 	if (err) {
@@ -1656,6 +1665,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		regulator_disable(rockchip->vpcie0v9);
 err_set_vpcie:
 	rockchip_pcie_disable_clocks(rockchip);
+err_disable_wake:
+	/* It's harmless to disable wake even not enabled */
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
 	return err;
 }
 
@@ -1682,6 +1695,8 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
 	if (!IS_ERR(rockchip->vpcie0v9))
 		regulator_disable(rockchip->vpcie0v9);
 
+	dev_pm_clear_wake_irq(dev);
+	device_init_wakeup(dev, false);
 	return 0;
 }
 
-- 
2.11.0

[PATCH v6 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>
---

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 | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 199a5118b20d..9e0269a13ced 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -707,7 +707,15 @@ 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", "wakeup";
+	/delete-property/ interrupts;
+
 	vpcie3v3-supply = <&pp3300_wifi_bt>;
 	vpcie1v8-supply = <&wlan_pd_n>; /* HACK: see &wlan_pd_n */
 	vpcie0v9-supply = <&pp900_pcie>;
@@ -722,11 +730,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

[PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> Add support for PCIE_WAKE pin in rockchip pcie driver.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> 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
>         -- Suggested by Brian Norris <briannorris@chromium.com>
> 
>  drivers/pci/host/pcie-rockchip.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 9051c6c8fea4..268513b6c9c4 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -37,6 +37,7 @@
>  #include <linux/pci_ids.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
>  #include <linux/reset.h>
>  #include <linux/regmap.h>
>  
> @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
>  		return err;
>  	}
>  
> +	irq = platform_get_irq_byname(pdev, "wakeup");
> +	if (irq >= 0) {
> +		/* Must init wakeup before setting dedicated wakeup irq. */
> +		device_init_wakeup(dev, true);
> +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> +		if (err) {
> +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> +			device_init_wakeup(dev, false);
> +		}
> +	}

There's nothing Rockchip-specific here, so I'm hoping you can explore
putting this support in the PCI core, so any system that describes the
WAKE# connection in the DT can benefit.

>  	return 0;
>  }
>  
> @@ -1123,10 +1135,6 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		return PTR_ERR(rockchip->clk_pcie_pm);
>  	}
>  
> -	err = rockchip_pcie_setup_irq(rockchip);
> -	if (err)
> -		return err;
> -
>  	rockchip->vpcie12v = devm_regulator_get_optional(dev, "vpcie12v");
>  	if (IS_ERR(rockchip->vpcie12v)) {
>  		if (PTR_ERR(rockchip->vpcie12v) == -EPROBE_DEFER)
> @@ -1155,7 +1163,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		dev_info(dev, "no vpcie0v9 regulator found\n");
>  	}
>  
> -	return 0;
> +	return rockchip_pcie_setup_irq(rockchip);
>  }
>  
>  static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
> @@ -1546,7 +1554,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  
>  	err = rockchip_pcie_enable_clocks(rockchip);
>  	if (err)
> -		return err;
> +		/* Cleanup rockchip_pcie_parse_dt() */
> +		goto err_disable_wake;
>  
>  	err = rockchip_pcie_set_vpcie(rockchip);
>  	if (err) {
> @@ -1656,6 +1665,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  		regulator_disable(rockchip->vpcie0v9);
>  err_set_vpcie:
>  	rockchip_pcie_disable_clocks(rockchip);
> +err_disable_wake:
> +	/* It's harmless to disable wake even not enabled */
> +	dev_pm_clear_wake_irq(dev);
> +	device_init_wakeup(dev, false);
>  	return err;
>  }
>  
> @@ -1682,6 +1695,8 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
>  	if (!IS_ERR(rockchip->vpcie0v9))
>  		regulator_disable(rockchip->vpcie0v9);
>  
> +	dev_pm_clear_wake_irq(dev);
> +	device_init_wakeup(dev, false);
>  	return 0;
>  }
>  
> -- 
> 2.11.0
> 
> 

[PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Brian Norris]

On Mon, Oct 16, 2017 at 03:03:50PM -0500, Bjorn Helgaas wrote:
> On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> > Add support for PCIE_WAKE pin in rockchip pcie driver.
> > 
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > ---
> > 
> > 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
> >         -- Suggested by Brian Norris <briannorris@chromium.com>
> > 
> >  drivers/pci/host/pcie-rockchip.c | 27 +++++++++++++++++++++------
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> > index 9051c6c8fea4..268513b6c9c4 100644
> > --- a/drivers/pci/host/pcie-rockchip.c
> > +++ b/drivers/pci/host/pcie-rockchip.c
...
> > @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
> >  		return err;
> >  	}
> >  
> > +	irq = platform_get_irq_byname(pdev, "wakeup");
> > +	if (irq >= 0) {
> > +		/* Must init wakeup before setting dedicated wakeup irq. */
> > +		device_init_wakeup(dev, true);
> > +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> > +		if (err) {
> > +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> > +			device_init_wakeup(dev, false);
> > +		}
> > +	}
> 
> There's nothing Rockchip-specific here, so I'm hoping you can explore
> putting this support in the PCI core, so any system that describes the
> WAKE# connection in the DT can benefit.

I guess it could work to look into pci_create_root_bus(), and
do something like the following?

	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
		... do OF parsing for generic features like WAKE# ...

Brian

[PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[Bjorn Helgaas]

On Tue, Oct 17, 2017 at 06:03:14PM -0700, Brian Norris wrote:
> On Mon, Oct 16, 2017 at 03:03:50PM -0500, Bjorn Helgaas wrote:
> > On Sat, Oct 14, 2017 at 03:50:56AM +0800, Jeffy Chen wrote:
> > > Add support for PCIE_WAKE pin in rockchip pcie driver.
> > > 
> > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > ---
> > > 
> > > 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
> > >         -- Suggested by Brian Norris <briannorris@chromium.com>
> > > 
> > >  drivers/pci/host/pcie-rockchip.c | 27 +++++++++++++++++++++------
> > >  1 file changed, 21 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> > > index 9051c6c8fea4..268513b6c9c4 100644
> > > --- a/drivers/pci/host/pcie-rockchip.c
> > > +++ b/drivers/pci/host/pcie-rockchip.c
> ...
> > > @@ -995,6 +996,17 @@ static int rockchip_pcie_setup_irq(struct rockchip_pcie *rockchip)
> > >  		return err;
> > >  	}
> > >  
> > > +	irq = platform_get_irq_byname(pdev, "wakeup");
> > > +	if (irq >= 0) {
> > > +		/* Must init wakeup before setting dedicated wakeup irq. */
> > > +		device_init_wakeup(dev, true);
> > > +		err = dev_pm_set_dedicated_wake_irq(dev, irq);
> > > +		if (err) {
> > > +			dev_err(dev, "failed to setup PCIe wakeup IRQ\n");
> > > +			device_init_wakeup(dev, false);
> > > +		}
> > > +	}
> > 
> > There's nothing Rockchip-specific here, so I'm hoping you can explore
> > putting this support in the PCI core, so any system that describes the
> > WAKE# connection in the DT can benefit.
> 
> I guess it could work to look into pci_create_root_bus(), and
> do something like the following?
> 
> 	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
> 		... do OF parsing for generic features like WAKE# ...

That's exactly the sort of thing I was thinking.

[PATCH v6 1/3] PCI: rockchip: Add support for pcie wake irq

[jeffy]

Hi guys,

On 10/18/2017 09:29 PM, Bjorn Helgaas wrote:
>> >
>> >I guess it could work to look into pci_create_root_bus(), and
>> >do something like the following?
>> >
>> >	if (IS_ENABLED(CONFIG_OF) && parent && parent->of_node)
>> >		... do OF parsing for generic features like WAKE# ...
> That's exactly the sort of thing I was thinking.
>
i was looking at that too, but i'm not very clear about how to handle 
the platform_ops's set_wakeup...